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

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

Open
casey-speer opened this Issue Apr 10, 2017 · 34 comments

Comments

Projects
None yet
@casey-speer
Copy link

casey-speer commented Apr 10, 2017

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:

  • VSCode Version: 1.11.1
  • VsCodeVim Version: 0.6.14
  • OS: MacOS 10.11.6
@johnfn

This comment has been minimized.

Copy link
Member

johnfn commented Apr 19, 2017

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

This comment has been minimized.

Copy link
Member

johnfn commented Apr 19, 2017

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

@Maistho

This comment has been minimized.

Copy link

Maistho commented Apr 23, 2017

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

This comment has been minimized.

Copy link
Member

johnfn commented Apr 23, 2017

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

@Maistho

This comment has been minimized.

Copy link

Maistho commented Apr 23, 2017

@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

This comment has been minimized.

Copy link
Member

johnfn commented Apr 23, 2017

@Maistho did my explanation (#1490 (comment)) not explain it? 😉

@Maistho

This comment has been minimized.

Copy link

Maistho commented Apr 23, 2017

@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

This comment has been minimized.

Copy link
Member

johnfn commented Apr 23, 2017

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

This comment has been minimized.

Copy link
Member

Chillee commented May 2, 2017

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

@bs

This comment has been minimized.

Copy link

bs commented May 10, 2017

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

This comment has been minimized.

Copy link
Member

Chillee commented May 10, 2017

@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

This comment has been minimized.

Copy link

bs commented May 10, 2017

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

@Chillee

This comment has been minimized.

Copy link
Member

Chillee commented May 16, 2017

@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

This comment has been minimized.

Copy link
Member

johnfn commented May 16, 2017

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

This comment has been minimized.

Copy link
Member

Chillee commented May 16, 2017

@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

This comment has been minimized.

Copy link
Member

johnfn commented May 16, 2017

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

This comment has been minimized.

Copy link

pluskid commented Aug 15, 2017

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

This comment has been minimized.

Copy link
Contributor

brandonbloom commented Sep 8, 2017

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

This comment has been minimized.

Copy link
Contributor

brandonbloom commented Sep 8, 2017

@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

This comment has been minimized.

Copy link

chuckdries commented Sep 12, 2017

Meanwhile...

Microsoft/vscode#20889

@chuckdries

This comment has been minimized.

Copy link

chuckdries commented Oct 18, 2017

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?

Zalastax/vscode#2

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

@koreyconway

This comment has been minimized.

Copy link

koreyconway commented Nov 1, 2017

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

This comment has been minimized.

Copy link
Member

Chillee commented Nov 1, 2017

@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

This comment has been minimized.

Copy link

masaeedu commented Nov 4, 2017

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!.

@ohjames

This comment has been minimized.

Copy link
Contributor

ohjames commented Nov 5, 2017

@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

This comment has been minimized.

Copy link

Zalastax commented Nov 11, 2017

@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.

@ohjames

This comment has been minimized.

Copy link
Contributor

ohjames commented Nov 11, 2017

@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

This comment has been minimized.

Copy link

Zalastax commented Nov 13, 2017

@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.

@ohjames

This comment has been minimized.

Copy link
Contributor

ohjames commented Nov 19, 2017

@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.

@mdunicorn

This comment has been minimized.

Copy link

mdunicorn commented Dec 16, 2017

@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": []
            }
        ]
    }
]
@ohjames

This comment has been minimized.

Copy link
Contributor

ohjames commented Dec 18, 2017

@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

This comment has been minimized.

Copy link

tiagoboldt commented Dec 9, 2018

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

This comment has been minimized.

Copy link
Member

Chillee commented Dec 9, 2018

@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.

@ohjames

This comment has been minimized.

Copy link
Contributor

ohjames commented Dec 9, 2018

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 it's entire undo handling will need to be rewritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment