formatOnSave scrolls away from current line on save

[Lucianod28]

With formatOnSave enabled, every time I save, my code gets formatted, but the editor window is scrolled down to a seemingly random location, so I have to scroll back up to find the line I was working on every time I save.

I'm using version v2023.4.1 of the extension and this is my full settings.json:

{
	"[python]": {
		"editor.defaultFormatter": "ms-python.black-formatter",
		"editor.formatOnSave": true,
		"editor.codeActionsOnSave": {
			"source.organizeImports": true
		}
	},
	"python.defaultInterpreterPath": "/usr/local/bin/python3",
	"black-formatter.importStrategy": "fromEnvironment",
	"isort.args": [
		"--profile",
		"black"
	]
}

[karthiknadig]

Can you provide an example code or video/gif of the issue?

[Lucianod28]

Can you provide an example code or video/gif of the issue?

Recording.2023-08-07.232144.mp4

Note I have relative line numbers. I hit save, it scrolls me down 30 lines, then I have to manually scroll back up to where I was.

[adamerose]

I have this too. It seems to happen only if you have this in your settings.json

  "editor.codeActionsOnSave": {
    "source.organizeImports": true
  },
  "editor.formatOnSave": true,

The two actions conflict and change the file twice, causing the number of lines to change twice and the scrollbar to shift down by that number of lines.

2023-09-10_23-00-03.mp4

[karthiknadig]

Duplicate of #118

[Lucianod28]

This seems different from #118 - the issue is not the caret moving, the caret stays in the same place but the window scrolls away from the caret.

[karthiknadig]

This extension actually does not control the UI, all it does is send back a diff in the form of TextEdit. VS Code calculates the View and the cursor position based on the diff.

The issue is with the diff itself that we are sending back from the server. Currently, we get the fully formatted content of the file from black and we return that as a single edit spanning the entire file. This is causing VS Code to move the cursor or to scroll to the wrong place. The solution is to generate narrow edits, which should address the problem.

[adamerose]

So the issue should be re-opened, right?

Also the problem is only reproducible when organizeImports is enabled. You can see in my clip that the imports get formatted on 2 lines for a split second and then reformatted again to 11 lines, and the difference (9 lines) is the distance the scroll position jumps every time. Not sure if this is consistent with your explanation or not.

[karthiknadig]

So the issue should be re-opened, right?

No. The solution to #118 and this are the same, from this extensions perspective. We need to provide narrow text edits.

Also the problem is only reproducible when organizeImports is enabled. You can see in my clip that the imports get formatted on 2 lines for a split second and then reformatted again to 11 lines, and the difference (9 lines) is the distance the scroll position jumps every time. Not sure if this is consistent with your explanation or not.

The fix to generate narrow edits has to be propagated to the import organizer as well. I will be adding this fix to isort and will let ruff know they need to make this update. The split second thing that you see is VS code getting a response back from import organizer (which run before full file formatting), the edits are applied to the editor buffer. Then the updated file content is sent to the formatter, the formatter returns edits, which are applied, and finally the file is saved.

[emchristiansen]

FYI, this is still an issue, and the upstream bug has been open since 2018.

Since they're probably never going to fix it, can we work around it?

[karthiknadig]

@emchristiansen We provided a mitigation for most scenarios. Please create a bug with a specific case that does not work. IT could likely be an editor bug.

You can also try Ruff extensions which now implements black formatting but in Rust. It should be more performant and can generate better results than using black directly as it is done here in this extension.

[emchristiansen]

Hey @karthiknadig,

Thanks for the quick response!

I took your suggestion to try Ruff, and it is fantastic so far - I doubt I'll use Black again.

[karthiknadig]

@emchristiansen Ruff extension uses the same extension template as this one. The only difference is that ruff (the tool itself) is so much faster than black that it can compute large diffs. Unfortunately, black does not give us diffs and we need to compute them manually, and that can take a lot of time. If the time to compute crosses a threshold (about 500ms) we skip and provide broad diff than narrow one, and it can cause the problem you see. We benchmarked diss between Ruff and black and you can see it here: astral-sh/ruff-lsp#290