VSCodeVim / Vim

:star: Vim for Visual Studio Code
http://aka.ms/vscodevim
MIT License
13.88k stars 1.31k forks source link

VSCodeVim Undo stack should be synced with VSCode undo stack #1490

Open casey-speer opened 7 years ago

casey-speer commented 7 years ago

VSCodeVim undo is not synced with VSCode undo. I've provided a couple different issues/test cases below to illustrate what I mean:

Simple undo VSCode and VSCodeVim seem to maintain different undo stacks, such that pressing u in VIM adds to the VSCode undo stack so that doing a VSCode undo (CMD-Z) undoes the VIM undo. u and CMD-Z should have the same function and share a common undo stack.

Search Replace When doing a vim-based search and replace, one change with all the replacements is pushed to the VSCodeVim undo stack, whereas one change for each replacement is pushed to the VSCode undo stack. It's easy to get confused about the state of the document if you make a replacement, then press CMD-Z thinking you've reversed all the changes, make some other changes, then have to backtrack to revert the rest of the replacements.

I'm not familiar with the extension architecture for VSCode, so I'm not sure what is technically possible here.

I can break these off into separate issues if you'd prefer.

thumbs-up 👍 this issue if it personally affects you! You can do this by clicking on the emoji-face on the top right of this post. Issues with more thumbs-up will be prioritized.


Simple undo

What did you do? Press i. Type some text: PIZZA. Press escape. Press i. Type some more text: IS GREAT. Press the u key to undo. Press CMD-Z.

What did you expect to happen? " IS GREAT" should disappear upon pressing u. "PIZZA" should disappear upon pressing CMD-Z so we're left with an empty string.

What happened instead? " IS GREAT" disappeared and then reappeared upon pressing CMD-Z so we're left with "PIZZA IS GREAT".

Search and replace

What did you do? Given a 50-line document with the string "Pizza" on each line, type %s/Pizza/Hot Dogs/ from the VSCodeVim command line. Press CMD-Z.

What did you expect to happen? "Pizza" should appear on every line.

What happened instead? "Hot Dogs" appears on every line except the last one which shows "Pizza". Similarly, if you first undo with u then press CMD-Z, "Pizza" appears on every line except the first one which shows "Hot Dogs".

Technical details:

johnfn commented 7 years ago

The basic explanation is this.

We could very easily map ctrl-z to u and ctrl-y to redo. We don't. Why?

We have u/Ctrl-r, and (to my untrained eyes) they seem to work basically perfectly. HOWEVER, as you noticed, we are maintaining our own undo history. We have to because it diverges from VSCode.

That means there is a naggling possibility there's still a bug somewhere that would lose your history soehow. Indeed, #1503 looks like one example. If there is such a bug, that would be REALLY BAD and you would lose work you finished. So rather than risk that 1% possibility, we just leave in the ctrl-z/y behavior, so that if we do screw up, you can still use them.

Feel free to leave feedback on whether this seems like a reasonable approach or not.

johnfn commented 7 years ago

I can see that the search & replace thing would be the cause of some confusion though. Let me know what you think.

Maistho commented 7 years ago

I think I would prefer it if u/ctrl+R just was mapped straight to the same commands as ctrl+Z/ctrl+Y.

What functionality is so important that VSCodeVim had to reimplement a history stack?

johnfn commented 7 years ago

@Maistho That would be history itself. 😄 Vim's history does not correspond with VSCode's in important ways.

Maistho commented 7 years ago

@johnfn okay 👌

I mapped u and C-r to undo/redo instead and it seems to work as it should imo. Not sure what I'm missing x)

johnfn commented 7 years ago

@Maistho did my explanation (https://github.com/VSCodeVim/Vim/issues/1490#issuecomment-295334612) not explain it? 😉

Maistho commented 7 years ago

@johnfn What I mean is that I bound u and C-r to vscodes built in commands (undo/redo) instead of the vim extensions undo/redo. Which means that I just don't use the vim extensions undo/redo functionality at all. :)

johnfn commented 7 years ago

Ohh, I misread, my mistake!

Trust you me @Maistho, if there is a single absolute truth in this treacherous world, it's that for every single Vim behavior, there is a person who will be very upset if that feature is off even by a single character. 😄 So yeah, we do have to do them the Vim way.

I'm glad that the VSCode commands work for you, though. That's awesome.

Chillee commented 7 years ago

@Maistho There's a couple big limitations with Vim's Ctrl-z functionality. One big example is macros.

bs commented 7 years ago

Could you explain in more details the differences between Vim's und VSCode's undo stack features? I'm curious to understand what causes the conflict in this case.

Chillee commented 7 years ago

@bs One example is macros. Following VSCode's undo stack, every ctrl+z press would undo one of the actions of the macro. However, in vim, you would want to undo the entire macro.

At the time undo functionality was implemented, VSCode didn't have the API features it has now. I believe VSCode added some new API features that allow us to interface with the undo stack, though.

bs commented 7 years ago

@Chillee Ah, fantastic. I'm not a huge user of macros, otherwise I would dive in. Looking forward to this being tackled though!

Chillee commented 7 years ago

@johnfn Do you think that if we set VSCode's undo stops to correspond with how they should be in vim, we can get rid of our undo stack completely?

It does mean that we'll never implement the vim undo tree though... 😏

johnfn commented 7 years ago

Yes, definitely.

That won't actually solve the problem in this issue however. Fixing this issue would essentially be a search and replace to replace "trigger a vscodevim undo step" with "trigger a vscode undo step". But the real problem here is that there are places where we should be triggering a step that we aren't.

Chillee commented 7 years ago

@johnfn wait what is it that simple? I've seen a bunch of buggy behavior related to undo that would probably be fixed by having vscode manage undo stops. Is there any advantage to having us manage undo behavior?

Is there actually a vscode "trigger undo stop" command we can call? I've only seen it around the edit API's.

johnfn commented 7 years ago

There are a fair amount of times where we use our undo history for something that vscode can't give us trivially. The biggest one that comes to mind is definitely stuff like jj to escape, which walks back a certain number of insertions (that may not have been stored in undo history).

But my approach is the basic strategy yeah.

pluskid commented 7 years ago

It is funny that I never remembered the shortcut for REDO in VIM, so whenever I try to undo and it undid a huge chunk of my code, I try to REDO, using the VSCode menu entry. And then the editor enters some weird state. Sometimes very confusing, and sometimes I end up losing a big chunk of my code. I have to say, this is very scary to use.

brandonbloom commented 7 years ago

At least for me, the currently broken behavior of undo right now is far worse than undo not undoing an entire macro execution. The behavior that happens in #2007 happens to me frequently and it is tantamount to corrupting my files. I am often left in a state completely unaware of how much was undone and whether or not I'm able to use vscode vim undo/redo to adequately recover from that.

brandonbloom commented 7 years ago

@Maistho How did you accomplish the remapping to bypass vscode vim's undo/redo? I tried vim.otherModesKeyBindingsNonRecursive and I can't get it to work.

chuckdries commented 7 years ago

Meanwhile...

https://github.com/Microsoft/vscode/issues/20889

chuckdries commented 7 years ago

Hey @johnfn, @zalastax and I are working on implementing undo branches in vscode's native history system. We'd like to make it possible for you to hook into us instead of needing to maintain your own history, what would an API look like that would allow you to do that without compromising on all the wonderful vim-ness that your fans demand?

https://github.com/Zalastax/vscode/issues/2

P.S. should I open a new issue for this here?

koreyconway commented 6 years ago

Why not just have a setting for users to decide which undo/redo system they want to use?

Something like vim.useNativeUndo or vim.useEditorUndo or vim.useSimpleUndo

My preference would be to default this to true and let more advanced users enable to get a more vim like undo system but as long as it is configurable then I'm happy.

Chillee commented 6 years ago

@koreyconway You can just bind u to <ctrl-z> and such.

@chuckdries I think for this project, it seems like it will be likely for us to implement the undo tree through neovim integration. I took a look at the vscode issue you linked, however, and the rendering looks great! Do you think it would be possible for us to leverage that somehow for displaying the undo tree?

masaeedu commented 6 years ago

One mildly annoying aspect here is that if you open a file and accidentally touch something, undoing will not result in the file being marked as "clean". VS Code thinks there are still unsaved changes in the file, so you can't close it without switching to :q!.

insidewhy commented 6 years ago

@masaeedu Yeah I noticed that, it has a few more implications than being slightly annoying to me and it's initially what lead me here. I've started to do some work on fixing it, with the current VSCode undo API it should be possible without too much trouble. I've made some progress but I want to get some other fixes I've made corresponding to COLEMAK in first.

Zalastax commented 6 years ago

@Chillee how does the neovim integration work? Does the integration replace the internals of VSCode to only make it a renderer? The UI only needs a fairly simple datastructure to traverse:

interface IHistoryElement {
    readonly index: number;
    readonly timestamp: number;

    readonly past: IHistoryElement | undefined;
    readonly futures: IHistoryElement[];
    readonly future: IHistoryElement;
 }

 export interface IHistory {
    readonly root: IHistoryElement;
    readonly now: IHistoryElement;
 }

enum HistoryEvent {
    Move,
    Change
}

/**
 * Move to an index in the history tree.
 */
moveTo(index: number): void;

/**
 * An event emitted when the history is changed.
 * @event
 */
onHistoryChanged(listener: (e: HistoryEvent) => void): IDisposable;

getHistory(): IHistory;

Making the UI integrate well with VSCodeVim is one of the primary ways forward to complete the pull request I have open. How would you like to interact with the tree in terms of keyboard commands etc? I have issues enabled in my fork so you can comment there if you don't want to clutter this discussion.

insidewhy commented 6 years ago

@Zalastax I've been doing some work on this in VsCodeVim... it maintains it's own undo stack separate from vscode. If you push ctrl-Z you get code's undo and "u" is mapped to the plugin's. I've been making VSCodeVim take advantage of the ability to suspend undo stops after/before edits, I currently do this during macro invocations and search/replace via :s.

There's probably other cases I need to be doing it but with these two (and with binding "u" to code's undo instead of the plugin's) I haven't noticed any undo issues... yet.

Zalastax commented 6 years ago

@ohjames so even if following @Chillee's suggestion of implementing the backend for the undo-tree in neovim there would be an undo stack maintained by VSCode? When thinking about what API the native undo tree should provide I need to know if VSCodeVim will completely replace the backend or just sync its state with the backend provided by VSCode. If the goal is to replace the backend with one provided by VSCodeVim then the API needs to allow the plugin to register an undo provider. If the goal is to sync the backend then the API should provide methods for replacing the internal state or at least modify and listen to it with enough granularity.

insidewhy commented 6 years ago

@Zalastax You could stop vscode creating an undo stack by passing { undoStopAfter: false, undoStopAfter: false } to every single edit API call. IMO creating a new undo stack separate from VSCode is going to eventually create a tonne more problems and work than it solves, I think we need to lobby for whatever undo stack improvements are necessary to VSCode's API.

prodehghan commented 6 years ago

@brandonbloom Probably a little late, but here is how you can map u and Ctrl+R to VS Code's native undo/redo:

updated for the v0.13.0 changes

"vim.normalModeKeyBindingsNonRecursive": [
    {
        "before": ["u"],
        "after": [],
        "commands": [
            {
                "command": "undo",
                "args": []
            }
        ]
    },
    {
        "before": ["<C-r>"],
        "after": [],
        "commands": [
            {
                "command": "redo",
                "args": []
            }
        ]
    }
]
insidewhy commented 6 years ago

@mdunicorn Yeah I'm using that config also, I have a change in the works that improves VSCodeVim behaviour when using undo/redo like this also, will open the PR in early Jan.

tiagoboldt commented 5 years ago

There hasn't been any activity in one year in this issue, but I think it is still a relevant problem. I often press u to see a way too large text block being changed. The plugin's history is not granular enough, which often results in a u to delete entire paragraphs. Really looking forward to a fix here.

Chillee commented 5 years ago

@tiagoboldt in some sense, that's partly because I think it's a somewhat different issue than the one reported here. The one reported here was a bug where users would press u, and VSCodeVim would undo 10s of minutes of work, sometimes even hours. That's horrendous.

On the other hand, the bug you're reporting seems more like a regular bug. Part of the problem is that It's difficult to troubleshoot a bug that's simply "the undo history isn't granular" enough, so a case where it does so would be very helpful.

insidewhy commented 5 years ago

When VSCodeVim's undo stack was implemented there was no ability to create (or not create) undo stops provided by VS Code's API. That API was implemented about a year ago. For VSCodeVim's undo support to sync with VS Code's its entire undo handling will need to be rewritten.

trusktr commented 5 years ago

Vim's history does not correspond with VSCode's in important ways.

Maybe provide an option. I'm sure many people could live with VS Code's history (I know, it may not properly reflect Vim actions, but I would be willing to live with that if it means I have consistency).

@mdunicorn Thanks for that solution! It makes undo/redo much more pleasant (consistent behavior).

J-Fields commented 4 years ago

I think this would be a heavy lift to do right, but I'll try to play around with it to get a prototype working in the near(ish) future. I think letting VSCode handle undo/redo for us would improve performance and consistency.

hamlittle commented 4 years ago

The March edition of VSCode introduced saving the undo/redo stack even after the file is closed. This was a feature I loved from VIM (saved undo/redo stack in a hidden file in my home directory), and sorely miss. Using the native undo/redo stack would get you this functionality for free. (I don't mean the work to do it is free, just that it would benefit from this and future features the undo/redo stack receive.)

In the meantime, I'm going to try the suggestion to re-map u and <ctl-r>.

chris-morgan commented 4 years ago

As a Vim user toying with VS Code, this difference has tripped me up multiple times within a few hours of starting using VSCodeVim, and I really can’t cope with the current behaviour (I’ll try the remapping because it sounds like it’ll be better, though differently imperfect in Vimminess). The most notable problems have been:

yunti commented 4 years ago

This is a painful issue I just came up against, as a relatively new vscode user. by hitting some weird combination of undo and cmd+z I managed to get in a weird loop that undoing or redoing wouldn't restore the file to it's original as the correct undo path had been lost somewhere between vscode's undo stack and vim's with using one then the other at various points.

J-Fields commented 4 years ago

@yunti Don't mix the two 🙂 They're unaware of each other and will mess one another up.

yunti commented 4 years ago

Yup don't cross the streams. :)

avishaan commented 4 years ago

Coming from vim, given that it seems I have to choose between ctrl z style and vim-u style, I would recommend at least defaulting to ctrl z and letting experienced vim users discover the alternate. I saw the post above about the 1% but the problem is for most of what I do, the current behaviour is not satisfactory. I'd rather have ctrl z be the default and do something else for vim-u when I need it

My point is from a DX perspective, the user story should fulfill the majority case given that the vscode has had to pick one implementation vs the other

docwhat commented 4 years ago

The interaction between the VSCode undo and the vim undo is very confusing; I've run and committed work that had weird errors in them because vim undo does too much and VSCode then seems to fix it, but it causes other issues off screen.

I'd rather have VSCode undo unless using neovim integration.

eddy-geek commented 3 years ago

Here's a sub-bug that I'm hoping can be fixed or worked-around: Using vscode's "Toggle Comment" (Ctrl+/) does not feed vim undo stack.

J-Fields commented 3 years ago

Here's a sub-bug that I'm hoping can be fixed or worked-around: Using vscode's "Toggle Comment" (Ctrl+/) does not feed vim undo stack.

This can be fixed by using the gc operator. You could map <C-/> to it, if you like that keybinding.

trusktr commented 3 years ago

There's a newer VS Code extension called Neo Vim that runs VS Code as Neovim's GUI (or think of it as all key strokes control a headless Neovim, and Neovim in turn controls the VS Code text buffers).

With this you get vim's actual undo/redo tree (not just a stack). I haven't tried it yet, but I suspect that vim's (neovim's) persistent history tree will also work (VS Code will have no idea it exists, neovim will simply be updating the text buffers in VS Code, so the history state effectively lives outside of VS Code and you never touch VS Code's history yourself).

In the following video I skip to the closing JSX (HTML-like) tag with % (works out of the box), I make three edits on the next line (thus the history tree has three branches thanks to Neovim), then I undo (u) and redo (<c-r>) a few times, and then I use g+ and g- to cycle through the three edits. Note that TypeScript (on the VS Code side, which Neovim has no idea exists) is performing the syntax checking and putting the red squigglies under the invalid syntax, and momentarily shows a tooltip with type information:

history-tree


The March edition of VSCode introduced saving the undo/redo stack even after the file is closed

@HamboLagos For me it only works if the file is re-opened in the same VS Code session. When I exit VS Code, the history is gone, and does not come back when I re-launch VS Code.

ViaxCo commented 3 years ago

For anybody still facing this issue, this setting gives me the desired behaviour:

  "vim.normalModeKeyBindings": [
    {
      "before": ["u"],
      "after": [],
      "commands": [
        {
          "command": "undo"
        }
      ]
    }
  ]
haberman commented 3 years ago

@J-Fields I noticed you un-assigned yourself from this issue. Would you be able to do a brain dump of the latest status, to help give a starting point in case someone else wants to pick this up?

In particular:

atulpatildbz commented 3 years ago

Followed suggession given by @ViaxCo and added the following mapping

  "vim.normalModeKeyBindingsNonRecursive": [    {
      "before": ["u"],
      "commands": ["undo"]
    },
    {
      "before": ["C-r"],
      "commands": ["redo"]
    }
  ]

Works fine. Does anyone see a risk with this workaround?

insidewhy commented 3 years ago

@atulpatildbz As has been mentioned several times in the thread you are replying to, this doesn't work with macros. There's a few other commands and things this doesn't work with also, tabdo, many more.