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

Format document sometimes adds blank newlines between every line #4865

Closed
DanTup opened this issue Nov 30, 2023 · 7 comments
Closed

Format document sometimes adds blank newlines between every line #4865

DanTup opened this issue Nov 30, 2023 · 7 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Nov 30, 2023

I've seen this a couple of times in the last few days:

format.mp4

My feeling is that it's related to having a document that is mostly LF but then you add some CRLF (for example during a merge conflict or pasting).

Here's a (zipped, to reduce the chance of anything messing with it) copy of the file from the video in the state that triggers it.

handler_execute_command.zip

And as I hit save, I reload VS Code to ensure it still occurs, but it does not. So probably there is some state involved and it's not just the file on disk. It's unclear if the issue is on the server or the client side.

@DanTup DanTup added is bug in editor Relates to code editing or language features labels Nov 30, 2023
@DanTup DanTup added this to the v3.80.0 milestone Nov 30, 2023
@BeezBeez
Copy link

BeezBeez commented Dec 6, 2023

It's been a while (some months) I encounter this problem too. The quickest fix I have found is to execute the command : Reload Window.

@BeezBeez
Copy link

BeezBeez commented Dec 6, 2023

I have just tested and switching the files to CRLF prevent the bug from happening but switching back to LF and the bug come back.

@DanTup
Copy link
Member Author

DanTup commented Dec 6, 2023

@BeezBeez do you know of any steps to reliably reproduce it?

I've tried writing some files with mixed line endings like:

void main(List<String> arguments) {
  File('lib/crlf.dart').writeAsStringSync('String?   a1;\r\nString?   a2;\r\nString?   a3;\r\n');
  File('lib/lf.dart').writeAsStringSync('String?   a1;\nString?   a2;\nString?   a3;\n');
  File('lib/mixed.dart').writeAsStringSync('String?   a1;\r\nString?   a2;\nString?   a3;\r\n');
}

However they all format normally and no extra lines are inserted.

@DanTup
Copy link
Member Author

DanTup commented Dec 6, 2023

Scratch that - I think I have a repro. I switched to the latest bleeding-edge Dart SDK (not stable) and it seems to trigger in my lf.dart. I'll debug :)

==> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///C%3A/Dev/Test%20Projects/dartcode_4865/lib/lf.dart","languageId":"dart","version":20,"text":"String? a1;\nString? a2;\nString? a3;\n"}},"clientRequestTime":1701873516421}
==> {"jsonrpc":"2.0","id":150,"method":"textDocument/formatting","params":{"textDocument":{"uri":"file:///C%3A/Dev/Test%20Projects/dartcode_4865/lib/lf.dart"},"options":{"tabSize":2,"insertSpaces":true,"trimTrailingWhitespace":true,"insertFinalNewline":true}},"clientRequestTime":1701873517989}
<== {"id":150,"jsonrpc":"2.0","result":[{"newText":"\r","range":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}}},{"newText":"\r","range":{"end":{"character":11,"line":1},"start":{"character":11,"line":1}}},{"newText":"\r","range":{"end":{"character":11,"line":2},"start":{"character":11,"line":2}}}]}

Edit: the repro stopped after restarting VS Code. Seems like there are some specific actions contributing to this. Same open file, same format request, different result:

==> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///C%3A/Dev/Test%20Projects/dartcode_4865/lib/lf.dart","languageId":"dart","version":2,"text":"String? a1;\nString? a2;\nString? a3;\n"}},"clientRequestTime":1701874095505}
==> {"jsonrpc":"2.0","id":163,"method":"textDocument/formatting","params":{"textDocument":{"uri":"file:///C%3A/Dev/Test%20Projects/dartcode_4865/lib/lf.dart"},"options":{"tabSize":2,"insertSpaces":true,"trimTrailingWhitespace":true,"insertFinalNewline":true}},"clientRequestTime":1701874096769}
<== {"id":163,"jsonrpc":"2.0","result":null}

Edit2: Ok, repros if I reload VS Code, save crlf.dart, then save lf.dart. Maybe something being cached is affecting the newlines between the files?

@DanTup
Copy link
Member Author

DanTup commented Dec 6, 2023

I figured this out, and I think there are two parts to it:

  1. The server reuses the formatter, which caches line endings. Formatting a CRLF doc and then a LF doc produces formatting that rewrites all the line endings from LF to CRLF.
  2. VS Code seems to show blank lines when a doc contains CRLF but is set to LF mode.

I'm working on a fix to the first (to never reuse the formatter), but have also opened dart-lang/dart_style#1337. I'll do some testing of the second issue and file a VS Code issue if appropriate after.

@DanTup DanTup added in lsp/analysis server Something to be fixed in the Dart analysis server relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available labels Dec 6, 2023
@DanTup
Copy link
Member Author

DanTup commented Dec 6, 2023

Dart fix is open at https://dart-review.googlesource.com/c/sdk/+/340220 and I've opened microsoft/vscode#200157 about the weird VS Code behaviour that resulted in the attempt to change LF to CRLF resulting in actual blank lines.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Dec 7, 2023
…atted with \n if a previous file was formatted with \r\n

Caching the formatter (something we recently discussed removing) was causing issues if you formatted multiple files with different line endings.

It might be a VS Code bug that "\r\n" is being rendered with a blank line in between when the document was in LF mode, but I'll investigate that separately and file an issue there if required.

Fixes Dart-Code/Dart-Code#4865

Change-Id: Ib5ec5355f078bc8dd2b30440fd253663727a2b66
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340220
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
@DanTup
Copy link
Member Author

DanTup commented Dec 7, 2023

Fixed by dart-lang/sdk@315d0b7.

The fix is in the analysis server so ships with a future Dart SDK. If you're often working on files with mix line endings, two possible workarounds:

  1. Restart the server... When you see the issue:
    1. Hit Undo to remove the extra lines
    2. Run the Save Without Formatting command to save the file
    3. Run the Developer: Reload Window command to restart the server
    4. Re-save/format the file
  2. Change the "dart.lineLength" setting to null
    The issue only occurs when the formatter was reused. Due to a bug, it was only reused if the line length was non-null and had not changed. Forcing it to null will still get the default of 80-character wrapping, but will bypass the cache:
    "dart.lineLength": null
    Note:: VS Code will tell you null is not a valid value.. You can ignore that (and after this fix ships in the next SDK, you can remove this setting)

@DanTup DanTup closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

No branches or pull requests

2 participants