Skip to content
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

Beautify Node.show() for parse tree printing #518

Closed
wants to merge 9 commits into from

Conversation

Dob-The-Duilder
Copy link

Rather than just indents, use an actual tree structure for better displaying syntax trees.

Before
image

After:
image

Copy link
Owner

@eliben eliben left a comment

Choose a reason for hiding this comment

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

This file is auto-generated -- see comment at the top. This change should be made in the generator -- _ast_gen.py

@Dob-The-Duilder
Copy link
Author

My mistake. Change has now been made in the _ast_gen.py file.

@eliben
Copy link
Owner

eliben commented Sep 6, 2023

Tests are failing

pycparser/c_ast.py Outdated Show resolved Hide resolved

children = self.children()
if len(children) > 0:
_, lastChild = children[-1]
Copy link
Owner

Choose a reason for hiding this comment

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

I think you should be able to accomplish this in a cleaner way using enumerate in the for loop below. Then you can trivially find if the index is the one before last and you won't need this condition and lastChild

Copy link
Author

Choose a reason for hiding this comment

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

I think I understood what you meant but if not let me know. Its been changed now.

pycparser/_ast_gen.py Outdated Show resolved Hide resolved
pycparser/_build_tables.py Show resolved Hide resolved
@eliben
Copy link
Owner

eliben commented Sep 9, 2023

Added some more minor comments; also - all the tests fail on CI, please fix

@eliben eliben closed this Mar 30, 2024
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.

None yet

2 participants