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: Fix sniper printing of comments when changing modifiers #3701

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented Nov 17, 2020

Fix #3697

This is heavily WIP, I don't have a fix yet. The problems are thoroughly described in #3697

This PR introduces a fix for the problem of disappearing whitespace between comments and type members with modified modifier lists. For example, given the following commented type member:

// this is a type member
private static int number = 2;

Adding a final modifier to the modifier list would result in the following print-out:

// this is a type memberprivate static int number = 2;

The source fragments will look something like so:

"// this is a type member"
["\n\t", "private", "static"]
...

Where the comment is a normal ElementSourceFragment and the list is a CollectionSourceFragment. The reason the whitespace isn't printed is that the it becomes part of the CollectionSourceFragment. The problem with this is that the modifier list is only entered when the first modifier is encountered, in this case private, and then the preceeding whitespace is printed. But as the CollectionSourceFragment hasn't been entered, the only preceeding whitespace is the comment (which is treated as whitespace).

The fix this PR does is to make the whitespace its own separate source fragment, like so.

"// this is a type member"
"\n\t"
["private", "static"]

This fix is only applied when the whitespace preceding a CollectionSourceFragment is the suffix whitespace of a comment. This is the only case where I've found it to be necessary.

Ps. This explanation may seem overly long, but I'm documenting this mostly for myself if this causes a bug tomorrow, and I've forgotten all about it.

@slarse
Copy link
Collaborator Author

slarse commented Dec 15, 2020

I've been working on this for half a workday now. I'm not sure I can fix this reasonably, there are simply too many corner cases with the way the sniper printer currently works.

@slarse
Copy link
Collaborator Author

slarse commented Dec 15, 2020

My new attempts at fixing this seem to have worked. It's really two entirely different problems. I'll write up an explanation tomorrow.

Ps. Not ready to merge, need to tidy up.

@slarse slarse changed the title WIP: Fix Sniper printing of comments followed by type member with changed modifier list review: fix: Fix Sniper printing of comments followed by type member with changed modifier list Dec 17, 2020
@slarse
Copy link
Collaborator Author

slarse commented Dec 17, 2020

This is ready for review, see the initial comment for an explanation.

@slarse
Copy link
Collaborator Author

slarse commented Dec 17, 2020

There is an issue related to this where removing the final element in a modifier will cause any preceding comment to disappear, but it's a separate implementation issue and so I'm treating it separately (see #3732).

@monperrus
Copy link
Collaborator

LGTM. Will merge. Thanks!

@monperrus monperrus changed the title review: fix: Fix Sniper printing of comments followed by type member with changed modifier list fix: Fix sniper printing of comments when changing modifiers Dec 22, 2020
@monperrus monperrus merged commit b6ec8a3 into INRIA:master Dec 22, 2020
@monperrus
Copy link
Collaborator

The sniper mode is ever better! Thanks @slarse

@slarse slarse deleted the issue/3697-sniper-type-member-comments-and-modifier-lists branch May 28, 2021 10:08
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.

bug: Sniper printer drops whitespace after comment if next element is type member with modified modifier list
2 participants