Skip to content

Add comment parsing from CastXML. #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Oct 23, 2020

Conversation

josephsnyder
Copy link
Contributor

In CastXML 3.6, a new feature was introduced to capture the
documentation comments added to the header files of the C++ sent through
the program. Add the code to pygccxml to read those new comment nodes and
attributes into a new "comment" declaration type.

In CastXML 3.6, a new feature was introduced to capture the
documentation comments added to the header files of the C++ sent through
the program.  Add the code to pygccxml to read those new comment nodes and
attributes into a new "comment" declaration type.
Copy link
Contributor

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as a starting point - thanks!

Would be nice if the declarations structure had some more contract enforcement.

My main nit here is that I'm not a huge fan of returning lines themselves. Can you instead retur the concatenated text, incorporating the columns too?

Add the additional properties from CastXML.
Add location as a proper property
Add type checks for the non-int properties.
Add __files_text to the scanner object.  It is a dictionary of file
declaration to the content of each file in a list of strings.

The first time a comment is a part of a file, read it in and store it in
the dictionary.  Each time afterwards, use the stored value to capture
the lines.

Use the (begin/end) columns to more precisely capture the text of each
comment.

Update the test to remove extra characters.
Expand the comment test to capture the comments found on variables in a
class.
Text taken from review suggestions.
@tom-osika
Copy link
Contributor

I saw CastXML also tests comments before namespaces, enums, etc. Would it be worth adding some of those here?

Expand the comment attribute reading to Namespace and Enumeration
objects.  Add tests for each of those objects.
Since the CastXML repository has the code for the comments but has not
generated a release yet, check the version of the castxml executable
before running.

If it is 0.3.6 or older, do not run the test.

Fix style errors.
@josephsnyder josephsnyder force-pushed the add_castxml_commenting branch from 25d1147 to c59db5c Compare October 13, 2020 14:07
@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented Oct 13, 2020

In conjunction with @tao558's comment, would you be able to see what comments look like for this kind of Christmas-tree test in Drake? (or rather, can you check that pygccxml can generally analyze the symbols that mkdoc can currently parse using clang.cindex?)
Code: sample_header.h
Example output: sample_header_documentation.expected.h

Stop trying to check the version with a regex and instead copy the
"Nightly" version of castxml available from data.kitware.com.
This Nightly version is master on October 14, 2020
@josephsnyder josephsnyder force-pushed the add_castxml_commenting branch from 67ab81b to 624ffcc Compare October 14, 2020 20:51
Switch from using a declaration as the parent class of a comment.
Instead, create a comment_t object for each declaration.
Enforce that the resultant object found in the comment property
is a comment_t object.
For new comment test, ensure that the text is found to be not empty
before checking content.  This should prevent failures on older versions
of CastXML.
Since comment_t no longer is a declaration_t child, add a new check in
startElement for a comment object.  Add it to a new dictionary just
for comments.

Remove leftover debugging statement.
Add a comment text checking function.  This print extra print signals
when the test passes thanks to the absence of strings as opposed to the
content matching.
Only use the start and end columns on the first and last line of the
comment.  Previously, a shorter line at the end would result in the
previous lines leaving information behind.
To be consistent, add the explicit check for "is not None" when checking
for the presence of a comment.
Use the start column of the comment on each line. Use the end column
only on the last line.
@billhoffman
Copy link

Is this ready to go now?

@josephsnyder josephsnyder marked this pull request as ready for review October 23, 2020 18:32
@josephsnyder
Copy link
Contributor Author

Yes, I believe that the backwards compatibility has been addressed and that it is ready for it's proper review

Copy link
Contributor

@tom-osika tom-osika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some very small changes but otherwise looks good

Fix spelling and  copy/paste errors in the documentation of the
properties of the comment_t object.
@iMichka iMichka merged commit ad6345c into CastXML:develop Oct 23, 2020
@iMichka
Copy link
Collaborator

iMichka commented Oct 23, 2020

Looks good. It's merged now. I'll let you use the latest develop version for a few days/weeks and we will cut a new release later if this OK for you?

@iMichka iMichka added this to the 2.1.0 milestone Oct 23, 2020
@josephsnyder
Copy link
Contributor Author

Thank you, @iMichka, that will be just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants