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

"Only format modified text" does not format the first line of modified text #3583

Closed
dcharkes opened this issue Sep 28, 2021 · 11 comments
Closed
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
Milestone

Comments

@dcharkes
Copy link

@DanTup would it be possible to get support for VSCode feature to only format changed chunks?

https://code.visualstudio.com/updates/v1_49#_only-format-modified-text

settings.json:

    "dart.enableSdkFormatter": true,
    "editor.formatOnSave": true,
    "editor.formatOnSaveMode": "modificationsIfAvailable",

This causes nothing to be formatted for me.

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label Sep 29, 2021
@DanTup
Copy link
Member

DanTup commented Sep 29, 2021

Do you have an example of changes not being formatted? It seems to work for me, although the first line doesn't seem to be formatted (this feels like a bug - if it's not the same thing you're seeing I'll open a new issue and fix, though if it's the same we can reuse this one).

Sep-29-2021 10-35-12

I presume you have Git (or other scm) enabled in VS Code? It only works if they're enabled in VS Code since that's how it gets the modified ranges (they show with a green bar in the gutter).

@dcharkes
Copy link
Author

Sorry for the slow response.

I ran into this when opening a workspace with two folders

	"folders": [
		{
			"path": "flutter/dev/bots"
		},
		{
			"path": "flutter/dev/docs"
		}
	],

https://github.com/flutter/flutter/tree/master/dev/bots
https://github.com/flutter/flutter/tree/master/dev/docs

Editing flutter/dev/bots/analyze_sample_code.dart

After hitting save nothing happens:

image

@dcharkes
Copy link
Author

dacoharkes@dacoharkes-linux:~/flt/flutter/dev/bots$ flutter --version
Flutter 2.6.0-12.0.pre.173 • channel pub-global-run-workaround • git@github.com:flutter/flutter.git
Framework • revision 4b96c18650 (2 weeks ago) • 2021-10-01 13:55:50 +0000
Engine • revision 2db4319447
Tools • Dart 2.15.0 (build 2.15.0-168.0.dev)
dacoharkes@dacoharkes-linux:~/flt/flutter/dev/bots$ dart --version
Dart SDK version: 2.15.0-168.0.dev (dev) (Thu Sep 30 12:23:13 2021 -0700) on "linux_x64"
Version: 1.61.0
Commit: ee8c7def80afc00dd6e593ef12f37756d8f504ea
Date: 2021-10-07T18:11:58.853Z
Electron: 13.5.1
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Darwin x64 20.6.0

Using VSCode remote from MacBook to a Linux cloudtop.

Extensions, disabled all local/remote extensions except for the Dart/Flutter and VSCode Remote extensions.

@DanTup
Copy link
Member

DanTup commented Oct 15, 2021

@dcharkes is it only the first line of the modified range that's not formatted? If you modify multiple consecutive lines and save, do the others format?

To support format range, we're diffing the formatted code from dart_style to generate smaller edits, then keeping only those within the range. This means the whitespace change that spans where the line starts is being dropped. I'll see if I can coe up with a way to handle this (though I want to be sure it's the only issue you're seeing, in case there's more).

@dcharkes
Copy link
Author

image

@DanTup
Copy link
Member

DanTup commented Oct 15, 2021

@dcharkes it's not obvious from the screenshot, but is it still doing nothing at all, or just producing incorrect formatting? Do you have Git enabled in VS Code (so the modified lines have a green mark in the gutter - perhaps that's what your green bg is)?

If you can share an exact sample file and log file, that might help me reproduce. You can capture a log with the Dart: Capture Analysis Server Logs command (best done on a small sample project - you can email logs to me at logs@dartcode.org if you don't want to attach here). Thanks!

@dcharkes
Copy link
Author

Sorry for not being clear, it's doing nothing at all.

Yes, the green background was from a diff-view.

Repro logs:
https://gist.github.com/dcharkes/d3d7c3af5f5b8a65652931d0a7275802
image

I do not see any "format" in the log at all.

@DanTup
Copy link
Member

DanTup commented Oct 15, 2021

Oh, it looks like you're not using LSP:

Analyzer type: DAS

Do you have dart.previewLsp: false in your setting? The original protocol doesn't support range-formatting, so it's only available over LSP. LSP should be enabled by default now, although I didn't remove previewLsp so those that disabled it to work around issues in the past may still have it disabled. I'll file an issue to rename that flag to something clearer, which will re-enable it for everyone unless the disable it with the new (non-preview) setting (although note that in the future, LSP will be the only option - the non-LSP code is not being maintained in the VS Code extension).

@dcharkes
Copy link
Author

That's it! Now it's only the first line that's not being formatted.

Thanks again for your help @DanTup !

@DanTup
Copy link
Member

DanTup commented Oct 15, 2021

Great! Let's keep this issue open to cover the first line issue then. Thanks!

@DanTup DanTup added in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug and removed is enhancement awaiting info Requires more information from the customer to progress labels Oct 15, 2021
@DanTup DanTup added this to the v3.28.0 milestone Oct 15, 2021
@DanTup DanTup changed the title Only format modified text "Only format modified text" does not format the first line of modified text Oct 15, 2021
@DanTup
Copy link
Member

DanTup commented Oct 26, 2021

I've got a CL open to address this at https://dart-review.googlesource.com/c/sdk/+/218141/.

@DanTup DanTup closed this as completed Oct 26, 2021
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Oct 26, 2021
…or ranges in LSP format range

Fixes Dart-Code/Dart-Code#3583.

Change-Id: Id2974b173eb3589ff983316d1cec0ea1e45ecd97
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218141
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
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
Projects
None yet
Development

No branches or pull requests

2 participants