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

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

Open
sbatten opened this issue Jun 2, 2020 · 3 comments
Open

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

sbatten opened this issue Jun 2, 2020 · 3 comments
Assignees
Labels
feature-request Request for new features or functionality undo-redo Issues around undo/redo
Milestone

Comments

@sbatten
Copy link
Member

sbatten commented Jun 2, 2020

Testing #98987

Repro:

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

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

@connor4312
Copy link
Member

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

  • 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 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)
  • Or maybe the language server--I know almost nothing about LSP details, apologies if this is naive--could provide a custom redo action. That is, the language server could say if the user tries to undo this action, then actually run rename(newName -> oldName) which would remove the need for vscode to be particularly smart for complex cases.

@sbatten
Copy link
Member Author

sbatten commented Jun 2, 2020

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
Copy link
Member

alexdima commented Jun 3, 2020

🐛 => 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.

@alexdima alexdima changed the title Could not undo all files because one is edited? Add partial cross-file undo if a file is changed in the meantime Jun 3, 2020
@alexdima alexdima changed the title Add partial cross-file undo if a file is changed in the meantime Partial cross-file undo if a file is changed in the meantime Jun 3, 2020
@alexdima alexdima added feature-request Request for new features or functionality undo-redo Issues around undo/redo labels Jun 3, 2020
@alexdima alexdima added this to the Backlog milestone Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality undo-redo Issues around undo/redo
Projects
None yet
Development

No branches or pull requests

3 participants