-
Notifications
You must be signed in to change notification settings - Fork 5
Introduce clang_utils.py, compilation_database.py and related changes #6
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
Conversation
|
Any updates here? |
|
Hey! |
8b93868 to
b6d3e58
Compare
|
@kunaltyagi Please have a look. |
kunaltyagi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang_utils.py is left. Will review it sometime soon
kunaltyagi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some documentation for CursorKind, Cursor and Type for a reader?
@kunaltyagi clang-bind/bindings/python/scripts/clang_utils.py Lines 107 to 134 in b3447ff
|
|
I'd put some link/see-also comment here: https://github.com/PointCloudLibrary/clang-bind/pull/6/files#diff-ed2336087d363ffe402008e9acaeeddb1fb4bebcaab2ed4e20b04fc0dd790035R103 And in the classes you've linked to, is there some official clang documentation that we can link to? If not, then please add 1 or 2 examples there |
Couldn't find official documentation, but added links to the python API and the description of checks. |
kunaltyagi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3/6 files ok. Will read the 3 remaining files in more detail soon
kunaltyagi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bindings/python/scripts/parse.py left. Need to review manually. GitHub shows a very difficult to read diff
| buildDir=compilation_database_path | ||
| ) | ||
|
|
||
| def get_compilation_arguments(self, filename=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filename required here? Shouldn't the db path in L9 solve this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filename for the cpp file to be parsed, to get that file's compiler args from the compilation database.
If none, it will return the compiler args for all the files in the compilation db.
kunaltyagi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved because the parse logic will be changed soon anyways
|
Any plans to clean up the commit history? |
Done 👍 |
|
@kunaltyagi Let's merge? |
Add utilities for some
cindex.py's classesUse those utilities to clean up
parse.pyUse class in parse.py and clean
Add compilation_database.py
closes #19