-
Notifications
You must be signed in to change notification settings - Fork 12.8k
prevented organizeImports appending an extra new line to declarations when end of a file #58986
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
base: main
Are you sure you want to change the base?
Conversation
… at end of a file relates microsoft#48126
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
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.
I like this change to organizeImports. I would like to test it out more, since altering the changeTracker would change the behavior of other services. @navya9singh would this affect moveToFile?
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.
Since textChanges
are sorted by position and non-overlapping, and we are only trying to prevent the text change at the very end of the file from extra newlines, we should be able to only perform this check on the last change.
I did try to do that originally but it's not always the last change entry. I noticed that there can be blank text entries in the change list which instruct removal of additional lines from the original source. So I ended up checking each change to be sure it would work. |
Do these extra removal lines only happen with organizeImports? |
I'm not sure if anything else remove lines. |
relates #48126
A visual of what this PR fixes:
I've updated existing tests that were forced to add an extra line due to this bug and added three new harness fourslash tests.
The new
organizeImports 24-26
tests in this PR prove that no extra EOLs are being appended by the server.There is still one additional issue left after this fix is applied. It occurs when having an EOL after an export declaration and executing the
organizeImport
command in vscode causes another EOL to be appended. (double EOL as visualized in #48126)I couldn't recreate this additional issue using the harness fourslash test engine. I can only recreate the double blank EOL when attaching to a vscode instance so my guess is that there is a bug in the vscode extension which is appending these other extra EOLs.