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 for edge case in which comments are dropped #1924

Merged
merged 2 commits into from
Aug 1, 2023
Merged

Conversation

petervdonovan
Copy link
Collaborator

Fixes vscode-lingua-franca/133.

In the issue, the comment // test would be completely erased from the following program by the formatter:

target C

main reactor Minimal {
  // test
}

This fixes that issue such that the program is instead formatted like this:

target C

// test
main reactor Minimal {
}

I realize that some people might expect the comment to remain inside the reactor, but I think that that would be too much work to implement without much obvious benefit. With the current design, the comment must be associated with some AST node in order to not be dropped; in this case, it is associated with the whole reactor (no other alternatives are available), and as a result, it goes above the reactor.

@petervdonovan petervdonovan force-pushed the do-not-drop-comments branch from 03a99fe to 2791e06 Compare July 28, 2023 01:36
@petervdonovan petervdonovan marked this pull request as draft July 31, 2023 16:58
@petervdonovan petervdonovan force-pushed the do-not-drop-comments branch from c2af4d0 to d08deab Compare July 31, 2023 18:45
@petervdonovan petervdonovan marked this pull request as ready for review July 31, 2023 18:45
The change on line 130 (filter out ancestor comments) is the relevant
one. This is just a matter of accidentally deleting one tiny bit of code
that was in master.
@petervdonovan petervdonovan force-pushed the do-not-drop-comments branch from d08deab to 0c0171c Compare July 31, 2023 19:04
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

LGTM!

@lhstrh lhstrh added this pull request to the merge queue Aug 1, 2023
Merged via the queue into master with commit 081351e Aug 1, 2023
@lhstrh lhstrh deleted the do-not-drop-comments branch August 1, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comments in empty reactors are removed when saving
2 participants