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

'u' cannot undo changes if the foucs on the current tab has been switched out and back. #1503

Closed
js-shi opened this issue Apr 13, 2017 · 18 comments

Comments

@js-shi
Copy link

js-shi commented Apr 13, 2017

  • Click thumbs-up 👍 on this issue if you want it!
  • Click confused 😕 on this issue if not having it makes VSCodeVim unusable.

The VSCodeVim team prioritizes issues based on reaction count.


What did you do?

Open several files in different tabs.
Edit in one tab.
Switch focus to any other tab.
Switch focus back to the modified file.
Press 'u'.

What did you expect to happen?

Undo the changes.

What happened instead?

Nothing happened.

Technical details:

  • VSCode Version: 1.11.1
  • VsCodeVim Version: 0.6.15
  • OS: Windows 7
@jleclanche
Copy link

This is annoying :( It's related to #1490; if that gets fixed, this should get fixed as well.

@johnfn
Copy link
Member

johnfn commented Apr 19, 2017

I can't reproduce this at all, but it does sound very bad. For me, undo histories are preserved correctly between tabs when I follow your repro steps. Could you please help me reproduce it with more details?

@jleclanche
Copy link

What's happening is that when you focus a different tab, then focus back to the first tab, the u undo stack has been lost. You can only hit Ctrl+Z to undo (but that's considered an operation in vim, which adds to the undo stack, and now u undoes the Ctrl+Z... all messy, better fix #1490)

@johnfn
Copy link
Member

johnfn commented Apr 19, 2017

I fully understand that, but when I follow the reproduction steps given, I cannot reproduce the bug.

@jleclanche
Copy link

:/ Not sure why that is.

Here's my config file in case that matters:

{
	"files.exclude": {
		"**/.git": true,
		"**/.coverage": true,
		"**/*.pyc": true,
		"**/.cache": true,
		"**/*.egg-info": true,
		"**/.tox": true,
		"**/.vagrant": true,
		"**/__pycache__": true
	},
	"search.exclude": {
		"**/node_modules": true
	},
	"editor.lineNumbers": "relative",
	"editor.cursorBlinking": "solid",
	"editor.insertSpaces": false,
	"editor.renderWhitespace": "boundary",
	"files.insertFinalNewline": true,
	"files.trimTrailingWhitespace": true,
	"python.linting.enabled": true,
	"python.linting.flake8Enabled": true,
	"python.linting.pylintEnabled": false,
	"python.pythonPath": "/home/adys/.config/Code/python.venv/bin/python",
	"vim.hlsearch": true,
	"vim.searchHighlightColor": "rgba(150, 150, 255, 0.3)",
	"vim.useSolidBlockCursor": true,
	"vim.useSystemClipboard": true,
	"vim.handleKeys": {
		"<C-k>": false
	},
	"vim.otherModesKeyBindings": [

	],
	"workbench.colorTheme": "Tomorrow Night"
}

@jeffschwartz
Copy link

I am experiencing this also. I tested this when switching focus to another tab, to the Explorer and to the Find dialog (cmd F) and they all broke undo. Can't be certain but undo will probably also break when switching to other VSC windows/panels/dialogs too.

Mac OS X 10.9.5
VSC 1.11.2
Plugin 0.6.16

My config settings follow:

// Place your settings in this file to overwrite the default settings
{
    // Enable / disable JavaScript validation. ** Note: Setting this to false requires using ESLint or some other linter.
    "javascript.validate.enable": false,
    // When enabled, will trim trailing whitespace when you save a file.
    "files.trimTrailingWhitespace": true,
    // Controls visibility of line numbers
    "editor.lineNumbers": "relative",
    // Controls indentation guides
    "editor.renderIndentGuides": true,
    // Controls visibility of the glyph margin
    "editor.glyphMargin": true,
    // Controls the font size.
    "editor.fontSize": 14,
    // Configure glob patterns for excluding files and folders.
    "files.exclude": {
        "**/.git": false,
        "**/.svn": true,
        "**/.hg": true,
        "**/.DS_Store": true
    },
    // Configure glob patterns for excluding files and folders in searches. Inherits all glob patterns from the file.exclude setting.
    "search.exclude": {
        "**/.git": true,
        "**/node_modules": true,
        "**/bower_components": true,
        "dist/**/*.js": true
    },
    // Migrated from previous "File | Auto Save" setting:
    "files.autoSave": "afterDelay",
    // Controls if the editor should automatically format the line after typing
    "editor.formatOnType": true,
    // Vim
    "vim.insertModeKeyBindings": [
        {
            "before": [
                "j",
                "j"
            ],
            "after": [
                "<escape>"
            ]
        }
    ],
    "vim.useSystemClipboard": true,
    // When there is a previous search pattern, highlight all its matches.
    "vim.hlsearch": true,
    "vim.useCtrlKeys": true,
    "editor.cursorStyle": "block",
    "window.zoomLevel": 0,
    "editor.cursorBlinking": "blink",
    "workbench.activityBar.visible": true,
    "window.titleBarStyle": "native",
    "terminal.integrated.fontSize": 16,
    "workbench.iconTheme": "vs-seti",
    "editor.minimap.enabled": false,
    "editor.minimap.renderCharacters": true,
    "editor.wordWrap": "on",
    "workbench.statusBar.visible": true
}

@js-shi
Copy link
Author

js-shi commented Apr 20, 2017

Well, here are the reproduce steps on my side for demonstration.

Open VSCode.
Create two new files through Ctrl+N and you'll get two tabs name untitled-1 and untitled-2.
Insert any word in untitled-1 such as "test".
Switch focus to untitled-2 using either mouse or Ctrl+PageDown.
Do nothing in tab untitled-2 and switch the focus back to untitled-1.
Press 'u' and the just inserted "test" should be removed while in fact nothing happened.

@jeffschwartz
Copy link

jeffschwartz commented Apr 21, 2017

I can confirm that this issue is still present in 0.6.17.
I can confirm that this issue is still present in 0.6.18.

@xconverge
Copy link
Member

@johnfn, problem found, and should be able to be fixed now, just not 100% the right way

We are actually creating a new modehandler whenever the tab is switched

c78e87b#diff-0bb19c54b9a7b2e95f54f5c7837131f5R79

If I remove that oldModeHandler.editor !== vscode.window.activeTextEditor then this problem doesn't happen.

An easy test to do is open 2 tabs, enter insert mode, switch tabs and switch back, it is in normal on my machine which is the way I knew a new modehandler was being created

I am afraid to touch it because this may break gd? Any ideas?

@johnfn
Copy link
Member

johnfn commented Apr 26, 2017

@xconverge are you sure that's the bug? that code has been in for a long time, and this bug appears to have manifested very recently.

Could you try a git bisect to see which commit introduces the bug? (Sorry - I would do it, but I can't repro it...)

@xconverge
Copy link
Member

That code has not been around for that long..it was added to fix gd

I am sure.

@johnfn
Copy link
Member

johnfn commented Apr 26, 2017

@xconverge the commit you linked to was added on march 3rd, almost 2 months ago, and I immediately pushed a new version when I fixed it. This issue was created about 2 weeks ago and blew up immediately. That's why I'm not so sure.

@xconverge
Copy link
Member

I am pretty sure though... I tested it when I referenced that commit

@Chillee
Copy link
Member

Chillee commented Apr 28, 2017

@johnfn I can't confirm it was exactly that commit that caused the issue, but at least as far back as April 4th (in the git history), this issue was present.

The fix that @xconverge posted works for me, although I can't say it if it breaks anything else (I also needed to comment out the oldModeHandler.dispose())

After that, VsCodeVim stops compiling for me.

@johnfn
Copy link
Member

johnfn commented Apr 28, 2017

@Chillee That's probably because we upgraded TypeScript around that point - but you should be able to force typescript to compile with errors if you just run it from the command line (tsc --watch)

@Chillee
Copy link
Member

Chillee commented May 2, 2017

@xconverge After testing it, removing those lines definitely breaks "gd". If you gd to a file that's already open, j and k don't give you the right cursor position.

@xconverge
Copy link
Member

xconverge commented May 2, 2017

Yes I am aware of that, but gd is only fixed with those lines in because we are constantly creating a new modehandler

@Chillee
Copy link
Member

Chillee commented May 2, 2017

@xconverge Yeah those lines of code definitely don't work as is. I was just noting that we can't just remove those lines and call it a day.

xconverge added a commit that referenced this issue May 4, 2017
Fixes #1503: Undo history isn't kept when switching tabs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants