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

Vim: Undo reselects previous change #7521

Closed
1 task done
riccardofano opened this issue Feb 7, 2024 · 4 comments · Fixed by #9317
Closed
1 task done

Vim: Undo reselects previous change #7521

riccardofano opened this issue Feb 7, 2024 · 4 comments · Fixed by #9317
Labels
defect [core label] vim

Comments

@riccardofano
Copy link
Contributor

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

The Undo action in vim mode is the exact same as Zed's default mode, it reselects the text you had selected on your previous change.
This differs from Neovim's behaviour of only moving the cursor at the start of the previously selected range.

Pressing u more than once using a build that contains this change #6948 will:

  1. Undo your change but reselect it
  2. Lowercase the now selected text
  3. Undo the lowercasing
  4. Repeat steps 2 and 3 ad infinitum

Steps to reproduce it

Use a recent version of Zed

  • Enter insert mode with i
  • Type HELLO then Escape
  • Press viwd to select what you just typed and then delete it
  • Keeping pressing u

Expected behaviour: the text to be gone after two u presses.

2024-02-07.21-56-05.mp4

Actual behaviour: the text keeps switching between HELLO and hello.

2024-02-07.21-58-46.mp4

Environment

Zed: v0.120.6 (Zed)
OS: macOS 13.5.2
Memory: 16 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

@riccardofano riccardofano added admin read Pending admin review defect [core label] triage Maintainer needs to classify the issue labels Feb 7, 2024
@ConradIrwin
Copy link
Collaborator

Thanks for writing this up. Undo and visual mode is a big wart in the vim emulation at the moment.

As you observe we just copy Zed's undo stack at the moment. I think we could likely improve this by returning to normal mode on undo, though we'd have to test it of course!

Getting better still will require us to remember which mode you were in (currently we can't distinguish between visual block, visual line, and visual on undo; so we need to store that on the selection that is written to the undo stack).

If you'd like to pair with me on some of this, please book time here: https://calendly.com/conradirwin/pairing!

@ConradIrwin ConradIrwin added vim and removed triage Maintainer needs to classify the issue labels Feb 8, 2024
@JosephTLyons JosephTLyons removed the admin read Pending admin review label Feb 8, 2024
@majorsylvie
Copy link

majorsylvie commented Feb 23, 2024

Sacrificing the ability to use u for lowercase in visual mode (without any additions requiring you to do do U -> ~ to force upper then toggle) the following keybinds allows me to chain u for undos:

[
  {
    "context": "Editor && vim_mode == normal && vim_operator == none && !VimWaiting",
    "bindings": {
      "u": "editor::Undo"
    }
  },
  {
    "context": "Editor && vim_mode == visual && vim_operator == none && !VimWaiting",
    "bindings": {
      "u": "editor::Undo"
    }
  },
]

bswinnerton added a commit to bswinnerton/zed that referenced this issue Feb 25, 2024
This commit fixes a bug that was introduced in zed-industries#6948
where undoing a visual selection would keep the editor in visual mode
which has a different binding for the `u` key, preventing a user from
continuing to undo until they hit escape.

This is not the most robust solution as it does not keep track of the
cursor position and restore it after an undo, but this is meant to be
a quick way of addressing the bug in zed-industries#7521.
@manuraj17
Copy link
Contributor

Just came across this issue. +1 for this.

@jgadbois
Copy link

I also run into this a lot

ConradIrwin added a commit that referenced this issue Mar 14, 2024
The important change here is to ensure that undo never lands you in
visual mode; but we also take care to restore the selection the same way
vim does (visual line goes to beginning of line, visual block to the top
left, etc.).

To help make this behaviour feel right we also group any deletions that
started insert mode with the first text inserted.

Fixes: #7521
ConradIrwin added a commit that referenced this issue Mar 14, 2024
The important change here is to ensure that undo never lands you in
visual mode; but we also take care to restore the selection the same way
vim does (visual line goes to beginning of line, visual block to the top
left, etc.).

To help make this behaviour feel right we also group any deletions that
started insert mode with the first text inserted.

Fixes: #7521

Release Notes:

- vim: Improved undo. It will now restore you to normal mode in the same
position as vim, and group deletions caused by `c` or `s` with the
concomitant insert.
([#7521](#7521)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect [core label] vim
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants