zed icon indicating copy to clipboard operation
zed copied to clipboard

Fix bug with vim mode undo of visual selection

Open bswinnerton opened this issue 1 year ago • 3 comments

This pull request fixes a bug that was introduced in zed-industries/zed#6948 where undoing an operation in vim mode would reselect the previous change in visual mode, which has a different binding for the u key, preventing the user from continuing to undo with the u key 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/zed#7521.~~

Release Notes:

  • Fixed undo of visual mode selections in Vim mode (https://github.com/zed-industries/zed/issues/7521).

bswinnerton avatar Feb 25 '24 21:02 bswinnerton

@bswinnerton Thank you so much for taking a pass at this, it's one of the big vim pain points right now.

If you'd like to pair with me to get this over the line you can book time here: https://calendly.com/conradirwin/pairing.

As you say, I think we need to fix this with tighter integration with the editor. There are a couple of different ways we could approach this, but I think the best might be to:

  • Add new EditorEvent enums for StartTransaction, Undo and Redo which expose the transaction ids and which the vim crate can listen to.
  • Inside the vim crate, on StartTransaction we maintain a map from tx_id to the vim mode at that time.
  • on Redo and Undo, we collapse the selections according to the mode that vim was in at the time.

We need to maintain the mode because the landing position of the cursor is different in each mode:

  • in Visual mode, each cursor collapses to the start of the selection (note on this branch we currently collapse to the head, which seems incorrect)
  • in Visual Line mode, each cursor collapses to the start of the line containing the selection
  • in Visual Block mode, we collapse the multi-selection into just the top-left corner.

ConradIrwin avatar Feb 27 '24 05:02 ConradIrwin

@bswinnerton let me know if you're still planning on taking a look at this; otherwise I'll put it back on my backlog

ConradIrwin avatar Mar 04 '24 23:03 ConradIrwin

Hey @ConradIrwin! I won't have a chance to work on this for another few weeks so feel free.

bswinnerton avatar Mar 05 '24 01:03 bswinnerton

:+1: Closing this for now. Race you for who gets to it first :D.

ConradIrwin avatar Mar 06 '24 05:03 ConradIrwin