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

Fix issue #2374 Comments of added Nodes are ignored in LexicalPreserv… #2390

Merged
merged 3 commits into from Oct 20, 2019
Merged

Conversation

jlerbsc
Copy link
Collaborator

@jlerbsc jlerbsc commented Oct 4, 2019

…ingPrinter

Fixes #2374 .

// In this case keyworld [class] is considered as a token and [A] is a child element
// So if we don't care that the node is an ExpressionStmt we could try to generate a wrong definition
// like this [class // This is my class, with my comment A {}]
if (node.getComment().isPresent() && node instanceof ExpressionStmt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the defining characteristic of nodes like ExpressionStmt that make them need behaviour like this? Maybe we can make that a property of the node, so we can avoid node specific exceptional logic here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's more specific to lexical printer than expression statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, but do you think there are other nodes likes this? If not, it's okay.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sincerely, I don't know! I don't know enough JavaParser to answer this question. Maybe the author could answer it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just merge it then :-)

@matozoid matozoid merged commit 2aa0945 into javaparser:master Oct 20, 2019
@matozoid matozoid added this to the next release milestone Oct 20, 2019
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.

Comments of added Nodes are ignored in LexicalPreservingPrinter
2 participants