Support preserving cursor location when reformatting a document

[DanTup]

Currently when you reformat a document the cursor remains at the same line/character. If there are significant changes above your cursor then this is quite confusing.

It would be great if DocumentFormattingEditProvider.provideDocumentFormattingEdits was supplied the current cursor location and able to return the new position after reformatting.

[jrieken]

It's not that easy because a document doesn't know anything about a cursor or multiple cursors. It's a concept only the editor knows. You should be able to listen globally to the onDidChangeTextEditorSelection event which is fired when the selection/cursor position of an editor changes. That also covers the case in which a document is open in multiple editors.

[DanTup]

That's why I posted it as a feature request; though I presume you don't like the idea of the formatter being provided editor information?

onDidChangeTextEditorSelection seems to be related to selection, but I'm just talking about cursor position.

Does it sound reasonable (reliable?) for me to get the active editor in provideDocumentFormattingEdits, check if its document is the same, stash the position, then re-set it after formatting?

[DanTup]

@jrieken I'm not actually sure if it's possible to implement this without support from VS Code?

We don't apply formats ourselves, all we do is return TextEdit[]. There doesn't seem to be any hook to use to set the cursor position after the formatting is applied?

Do you not think this is a reasonable feature? When formatting a file that results in lots of changes you end up looking at completely different code to where you were and it's mighty confusing.

[jrieken]

So, generally VS Code tries to keep the cursor position stable. I'd like to learn more about your specific case, esp what language you have, what text your format, and what the edits are like. There is a lots of logic to keep the cursor(s) stable and maybe you are just hitting a bug?

[DanTup]

My extension is for Dart. When I reformat (code here) the cursor seems to end up on the same line number/character as before the format as if no attempt was made.

I'm not at PC so I'm typing this a bit blind. I'll test/update when I am, but this behaviour was so consistent I suspect it'll show the issue fine.

• Install that extension
• Create a folder with a .dart file
• Enter the contents below
• More your cursor to next to the closing brace
• Reformat

I suspect your cursor will remain on the same line number, not remain with the closing brace.

Sample contents

main()




{
  var i = 1;
}




// dummy

The API I'm using to format the code can provide me the new cursor location (if I provide it one) since it has all the intelligence about what was changed. It would be nice to be able to provide this to Code in preference to its own logic.

[jrieken]

Thanks - I understand what's going on. Just unsure how to go on from here ;-) The issue is that a single, none minimal, edit which replaces the whole content is returned. That makes the editor lose its cursor position. Unsure where the minimal edit (merge end of line #1 with start of line #6) should be computed... @alexandrudima ideas?

[DanTup]

If the user is passing the reformatting out to an external service it's always possible they'll get a huge edit back (if I shelled out to dartfmt it'd always be the entire file, though I do expect this service to be better than that) so having the ability for the language service to override it seems a much better idea than Code trying to do the best thing for all languages? (I appreciate it's more work, but it allows extensions to give a better experience? :))

[jrieken]

Nah, smaller edits are always better for various reasons like keeping multiple cursors stable (instead of them all collapsing), nice undo behaviour, and better changes for incremental compilers. We could invest in running a diff on the text to be replaced and the text to be inserted such that we can make more minimal edits. Since that's a big ask for extension writers we could do that on our side

[DanTup]

I think a diff would be better but it seems to me that the processor doing the reformatting is far better placed to provide an accurate location (it did after all already parse the document and know exactly what was changed). My language service already has this functionality, I just can't use it.

Granted, I only have support for one cursor; but if Code was able to use this data then maybe I could get the Dart team to consider adding support for sending an array instead of a single location and I could map them all cleanly.

[jrieken]

My language service already has this functionality, I just can't use it.

Yes - the language brain is usually suited best for these kind of task and it should produce a minimal edit instead of saying 'replace all with this text'.

Granted, I only have support for one cursor; but if Code was able to use this data then maybe I could get the Dart team to consider adding support for sending an array instead of a single location and I could map them all cleanly.

Not sure what that means? The formatting operation should be independent of cursors but only describe edits

[DanTup]

Not sure what that means? The formatting operation should be independent of cursors but only describe edits

The formatting is independent of the cursor location but the new cursor location is somewhat dependent on the formatting. The service I'm using to format also takes an optional cursor location and will return the new cursor location after the format.

Yes - the language brain is usually suited best for these kind of task and it should produce a minimal edit instead of saying 'replace all with this text'.

This is very valid.

I guess I'm stuck at the moment - Code wants my reformat in small exact changes but the Dart analysis service provides bigger changes and a cursor mapping. These are somewhat incompatible. Unfortunately neither of these are my code, so the options seem to be:

1. Convince you to allow the cursor position to be mapped by the formatter
2. Convince Google to break down their formatter output further
3. ???

I think 1 is least work, but may affect your API in ways you might not like.

[alexdima]

A bit late to the party, but here are my thoughts:

It is desirable of any formatting edits to be as minimal as possible (preferably the edits only target whitespace). @DanTup some reasouns:

• imagine you have an extension that uses decorations (e.g. bookmarks) that eventually are implemented as markers (same as cursors) -- a large formatting edit will mess up all the markers, not just those used by the cursors (i.e. all your bookmarks are now "lost")
• imagine you have fileA opened in two editors, side-by-side. If the format in editor1 returns the resulting cursor position, the cursor position in editor1 would be correct, while in editor2 it would be "lost".
• imagine you have 20 cursors (multicursor) in a file and trigger format. Should we impose that all languages become aware of multi-cursor. i.e. - is gofmt aware of multi-cursor when providing a resulting cursor position.

That being said, my recommendation is to enrich the gofmt tool for this use-case (usage inside an editor). Perhaps it was designed for usage on the command line? In my experience, given an AST, generating minimal edits should be sort-of-easy effort at that level. If this is not possible, then it is possible to compute a diff of the original and the desired result and produce "minimal" edits by post-processing the one resulting edit.

We could do the diff on our side, but IMHO the right fix is in generating minimal edits straight from the AST. We can brainstorm on adding diffing API in vscode, but I would not add the diff on the UI side.

[DanTup]

@alexandrudima Thanks; this all sounds pretty sensible to me. I'm not actually using dartfmt as a command line tool, I'm interacting with a real Dart analysis service (however it seems the formatting isn't returning the data as you would like, which is also a surprise to me).

I posted in the Dart analyzer group with details and a link here and I hope they might consider changes to reduce the size of the edits.

[alexdima]

Ok, let's keep this issue around as well until we figure out a solution.

[jrieken]

fyi - I have started working on making text edits returned by extensions more minimal: #10310

[DanTup]

That's great! There are comments from a Dart dev in the linked thread; seems like they may also open to changes (and also suggested a similar diffing solution if changing the formatter is non-trivial).

[jrieken]

[DanTup]

This looks great! Are you going to ship this, or is it just a prototype? Do you have any concerns over performance implications of the tidying up?

[jrieken]

It's in master so also in the next Insiders Release and the August Release. Since it's early in the month we have plenty of time to sand off rough edges but I am confident. We might add some self defence guard in case changes are huge...

[DanTup]

Great; thanks! (for this and all the other help/responses lately!) 👍

[jrieken]

You are welcome - thanks for the feedback and your cool extension.