microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.05k stars 29.23k forks source link

Partial cross-file undo if a file is changed in the meantime #99159

Open sbatten opened 4 years ago

sbatten commented 4 years ago

Testing #98987

Repro:

  1. Rename symbol to update 3+ files
  2. Edit one file on disk
  3. Undo on another file

:bug: => cross-file undo does not work at all. Could we fix a majority of the files that don't have this issue?

connor4312 commented 4 years ago

I ran into quite a few problems trying to test around this.

sbatten commented 4 years ago

The undo fails regardless of the change made in the other file. I wonder if VS Code could be smart enough to see that the range effected by the edit/undo wasn't changed, and be able to handle the rename (even line-level detection would probably work)

felt this way as well

alexdima commented 4 years ago

🐛 => cross-file undo does not work at all. Could we fix a majority of the files that don't have this issue?

That is a valid idea, but IMHO would be slightly worse. For example, to undo a variable rename in 10 out of 15 files would make it likely that many users won't realize which files have been missed by the undo. IMHO, people expect that cross-file operations (basically edits which people cannot see) should execute only under perfectly safe conditions. For example, doing a rename where TypeScript only renames an unpredictable number of usages (but not all usages) is not acceptable. IMHO it is better for the editor to be predictable and fail fast than do something surprising. Running the undo in the focused file is still unsafe, since it basically introduces very many compile errors in such cases, but IMHO this is the slightly less surprising default, because users can immediately see the effects of the undo operation since they have the file opened. This is consistent with IDEs I have tried like eclipse.

The undo will only happen in the single file if any file is changed. But I can't redo, un-change any changed files, and try to undo again. Effectively, if I undo and there's any change in other files, my ability to undo is permanently lost. From back when I used Goland/PHPStorm, think JetBrains' stuff will prompt whether you want to apply a partial undo or cancel the action.

The undo stack is not something that you can really see, so trying to go to other files that were edited in the meantime and hitting Cmd+Z a specific number of times to stop right before the cross file undo is IMHO unrealistic. The ability to undo is not permanently lost. The ability to undo in one step across all files is permanently lost. It is still possible to open each and every file and undo locally as much as needed. I acknowledge that you would like a prompt between local / partial / cancel.

The undo fails regardless of the change made in the other file. I wonder if VS Code could be smart enough to see that the range effected by the edit/undo wasn't changed, and be able to handle the rename (even line-level detection would probably work)

This is its own feature request, please create a new issue. The ability to undo a specific element in the undo stack (not the most recent one) is something people have asked about before and I think it deserves its own issue.

Or maybe the language server--I know almost nothing about LSP details, apologies if this is naive--could provide a custom redo action.

This is a good idea, but there are certain expectations that it would break. For example, if there are many compile errors at the time of the undo, many symbols might be missed by the "undo rename". The current textual based undo has the contract that it will bring the file(s) back to a state that existed in the past.


Finally, thanks for the good feedback.

I suggest the following:

  1. we keep this issue to implement partial undo and a prompt to be able to pick it in this case.
  2. @connor4312 makes a new issue for the "random access" undo.