Topology2 doxygen documentation infrastructure #8141
Conversation
jsarha
force-pushed
the
topology2-doxygen
branch
from
e28a0c1 to
f598c94
Compare
jsarha
changed the title
|
Turned this ready for review to gather more attention, but I still intent to turn at least the most relevant pieces of existing comments into Doxygen documentation before this is ready to merge (in any case it hardly is that yet). If one wants to generate the documentation do following steps:
|
@jsarha thank you for starting this. Looks good to me. Just a few typos to fix |
jsarha
force-pushed
the
topology2-doxygen
branch
from
f598c94 to
6db0b96
Compare
|
jsarha
force-pushed
the
topology2-doxygen
branch
from
6db0b96 to
c2a3932
Compare
|
Some more comments changed to doxygen. Updated the contents page. Its now autogenerated, but unfortunately I could not get cmake to generate it automatically. Any idea how to do it? |
jsarha
changed the title
| USES_TERMINAL | ||
| ) | ||
|
|
||
| add_custom_target("doc-clean" |
There was a problem hiding this comment.
Unfortunately you need BOTH add_custom_target() AND add_custom_command() for each target every time. Do not put any commands in add_custom_target(), add them to add_custom_command() only.
This really sucks but it's required. Best explanation why:
There was a problem hiding this comment.
Did not have time to do this yet. Will do it at some point,
There was a problem hiding this comment.
Don't define "clean" targets:
- CMake does this automatically when you use it right
- For "distclean" the only reliable, official and documented CMake way is to delete the entire build directory.
| print(str); | ||
| return True | ||
| return False | ||
| # Parses topology2 Define { } block and outputs C-preprocessor #define macros |
There was a problem hiding this comment.
Use pydoc syntax for this.
Also use pylint.
| # find the pieces of related Doxygen documentation. The filter also | ||
| # creates separate pages of the original code and adds links next to the | ||
| # pages in the C struct definition, instance documentation and their | ||
| # possible Doxygen documentation. |
There was a problem hiding this comment.
This look like implementing a new parser entirely from scratch? Have you considered using a framework?
https://tomassetti.me/parsing-in-python/
https://stackoverflow.com/questions/6571964/which-tool-to-use-to-parse-programming-languages-in-python
There was a problem hiding this comment.
When I started, I did not yet know what I need. I first thought that I can just translate everything as I go through the file, with very little state information. Parts of the translation still works like that, but then there are many places where more state is needed and bigger chucks has to be processed statefully.
If I would start from scratch now, I would probably use some more advanced method, but this is now here and it pretty much does what is needed, so I leave that excercise to a later time.
There was a problem hiding this comment.
If I would start from scratch now, I would probably use some more advanced method, but this is now here and it pretty much does what is needed, so I leave that excercise to a later time.
Please add this "technical debt" warning comment at the top of the file so this does not become a time sink for someone else.
There was a problem hiding this comment.
Ping.
Also: please run pylint and fix at least the scariest warnings (I haven't reviewed the Python code, too long)
jsarha
force-pushed
the
topology2-doxygen
branch
2 times, most recently
from
7864b71 to
a1f00e6
Compare
|
jsarha
force-pushed
the
topology2-doxygen
branch
from
a1f00e6 to
79c46d6
Compare
|
Some minor typofixes. And dropped an idle commit of spurious change. |
| # find the pieces of related Doxygen documentation. The filter also | ||
| # creates separate pages of the original code and adds links next to the | ||
| # pages in the C struct definition, instance documentation and their | ||
| # possible Doxygen documentation. |
There was a problem hiding this comment.
Ping.
Also: please run pylint and fix at least the scariest warnings (I haven't reviewed the Python code, too long)
|
jsarha
force-pushed
the
topology2-doxygen
branch
from
79c46d6 to
d04451e
Compare
That should be one or two lines.
CMake always tends to be a verbosity and complexity nightmare even for the simplest things and I don't see the CMake part of this PR converging quickly. That's why I'm asking to postpone it to the next PR not to hold this one back.
Yes please but it does not have to be a different script, I think you can just add it to the existing one. Or it can be a different and very short one. This will be much faster because: 1. it should be short either way 2. shell scripting is less powerful but much simpler than CMake 3. Like most people, you seem comfortable with the former but not the latter. |
Don't we have somthing that works with CMake now with the required fixes identified above (and some can even be click to merge) ? |
This is the second version of topology2 to C Doxygen filter. Its far from perfect, but it should get us started. The purpose of the translated C code is not to document actual topology2 code, but only to provide anchors for Doxygen to form a network of links through which to navigate the topology sources and find the pieces of related Doxygen documentation. The filter also creates separate pages of the original code and adds links next to the pages in the C struct definition, instance documentation and their possible Doxygen documentation. Signed-off-by: Jyri Sarha <[email protected]>
Includes all *.conf files under topology2 to the Doxygen documentation and points directly to mainpage Doxygen source. Signed-off-by: Jyri Sarha <[email protected]>
jsarha
force-pushed
the
topology2-doxygen
branch
from
eec1886 to
4b392e5
Compare
|
| ) | ||
|
|
||
| add_custom_target("doc-clean" | ||
| COMMAND rm -rf ${PROJECT_BINARY_DIR} |
There was a problem hiding this comment.
This target is literally longer to type and use than the command it runs. This is not the CMake way (which already generates a "clean" target)
There was a problem hiding this comment.
Can't call it just "clean":
CMake Error at CMakeLists.txt:49 (add_custom_target):
The target name "clean" is reserved or not valid for certain CMake
features, such as generator expressions, and may result in undefined
behavior.
I'll drop the target all together. it should not be needed for anything, now that the dependencies are handled correctly.
|
Can't finish the job today. I'll go to office and lab now, and I do not have time this evening, but I try to pull this together tomorrow. |
Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
jsarha
force-pushed
the
topology2-doxygen
branch
from
4b392e5 to
18b4aa7
Compare
|
jsarha
force-pushed
the
topology2-doxygen
branch
from
18b4aa7 to
53a9e6f
Compare
|
Oh, I fogot yesterday the cat << EOF change. No that is added too. I think I've now addressed all the issue pointed by @marc-hb . |
…ntents This commit adds topology2-generate-contents.sh the script to generate the contents page that will be inline included to the mainpage added in the next commit. Signed-off-by: Jyri Sarha <[email protected]>
The initial main-page contains a short introduction into topology2 Doxygen documentation, and an inline reference to automatically generated contents page. The commit also adds more advanced cmake rules to track all document dependencies. Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
Improve and convert comments to doxygen documentation. Signed-off-by: Jyri Sarha <[email protected]>
Doxygenify all comments that are correctly placed near the attribute's
or objects C-translation. This is not the case for comments before
Object.Base { } definition or comment before including
pipeline-common.conf, which is inlined by the topology2 filter, does
not end up in any relevant place, so its better just leave those
comments as they are. They can anyway be read from topology2 source
snippets.
Signed-off-by: Jyri Sarha <[email protected]>
Change attribute comments to Doxygen comments. Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
Signed-off-by: Jyri Sarha <[email protected]>
There were couple of classes that did not have their topology2 snippets showing. Probably the essential problem was just the class definition starting from the first line of the file. There was nothing obviously wrong in the output of the filter, but the way the doxygen works is sometimes hard to anticipate. Adding \struct before the definition fixed the problem. Signed-off-by: Jyri Sarha <[email protected]>
jsarha
force-pushed
the
topology2-doxygen
branch
from
53a9e6f to
cf09373
Compare
|
Oh, and couple of other contents scripts changes too, and their dependency to CMakeLists.txt. But now, this should be good. |
Now this starts to be a little closer where it should be. There is still a lot of comments to be turned into Doxygen documentation, not to mention, documentation to be written for completely undocumented topology code. I will at least convert the existing comments as soon as the generic appearance of this exercise is accepted.