Skip to content

helix: Fix line selection after undoing delete - #55365

Merged
tomhoule merged 13 commits into
zed-industries:mainfrom
GoldStrikeArch:fix/helix-correct-delete-then-undo-state
Jun 5, 2026
Merged

helix: Fix line selection after undoing delete#55365
tomhoule merged 13 commits into
zed-industries:mainfrom
GoldStrikeArch:fix/helix-correct-delete-then-undo-state

Conversation

@GoldStrikeArch

Copy link
Copy Markdown
Contributor

Closes #55170

Fixes Helix mode undo selection history after d deletes the character under an empty cursor. Previously, d temporarily expanded the cursor to a one-character selection before deleting, and undo restored that temporary selection. When the deleted character was a newline, pressing x after undo selected additional line which was wrong.

This preserves the original Helix selection as the undo-restored selection while keeping the existing delete behavior unchanged.

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Release Notes:

  • Fixed Helix mode selecting one extra line after undoing a newline delete

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 30, 2026
@ChristopherBiscardi ChristopherBiscardi added the area:parity/helix Feedback for Helix parity features label May 5, 2026
@tomhoule tomhoule self-assigned this Jun 5, 2026
Comment thread crates/vim/src/helix.rs

cx.set_state("oneˇ\ntwo\nthree", Mode::HelixNormal);
cx.simulate_keystrokes("d u x");
cx.assert_state("«one\nˇ»two\nthree", Mode::HelixNormal);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the expected behaviour cx.assert_state("«oneˇ»\ntwo\nthree", Mode::HelixNormal); (selection and cursor on the same line)? I just checked in Helix. This assertion seems to match what I see in Zed (current main).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the cursor is on a newline character but the visible selection is on a character before it. This is how it is right now (somewhat hacky implementation of a helix mode itself). I had some proposal how all of that could be refactored here, if you are interested -> #56768

This assertion seems to match what I see in Zed (current main).

In the current main branch you will end up selecting both one and two in this example:

oneˇ
two
three

I think it was this commit which is responsible for it -> 81da953

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more precise in our concrete example it would be this code -> https://github.com/zed-industries/zed/blob/main/crates/editor/src/element.rs#L152 but its all triggered because of this:

 cursor_offset_on_selection: self.mode.is_visual() || self.mode.is_helix(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I probably wasn't using the right sequence, I can see the difference with main now.

@GoldStrikeArch
GoldStrikeArch requested a review from tomhoule June 5, 2026 15:33

@tomhoule tomhoule left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an improvement, and the implementation makes sense. Thanks for the contribution!

@tomhoule
tomhoule added this pull request to the merge queue Jun 5, 2026
Merged via the queue into zed-industries:main with commit befa775 Jun 5, 2026
34 checks passed
TomPlanche pushed a commit to TomPlanche/zed that referenced this pull request Jun 8, 2026
Closes zed-industries#55170

Fixes Helix mode undo selection history after `d` deletes the character
under an empty cursor. Previously, `d` temporarily expanded the cursor
to a one-character selection before deleting, and undo restored that
temporary selection. When the deleted character was a newline, pressing
`x` after undo selected additional line which was wrong.

This preserves the original Helix selection as the undo-restored
selection while keeping the existing delete behavior unchanged.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed Helix mode selecting one extra line after undoing a newline
delete

Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>
This was referenced Jun 18, 2026
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
Closes zed-industries#55170

Fixes Helix mode undo selection history after `d` deletes the character
under an empty cursor. Previously, `d` temporarily expanded the cursor
to a one-character selection before deleting, and undo restored that
temporary selection. When the deleted character was a newline, pressing
`x` after undo selected additional line which was wrong.

This preserves the original Helix selection as the undo-restored
selection while keeping the existing delete behavior unchanged.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed Helix mode selecting one extra line after undoing a newline
delete

Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
Closes zed-industries#55170

Fixes Helix mode undo selection history after `d` deletes the character
under an empty cursor. Previously, `d` temporarily expanded the cursor
to a one-character selection before deleting, and undo restored that
temporary selection. When the deleted character was a newline, pressing
`x` after undo selected additional line which was wrong.

This preserves the original Helix selection as the undo-restored
selection while keeping the existing delete behavior unchanged.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed Helix mode selecting one extra line after undoing a newline
delete

Co-authored-by: Tom Houlé <13155277+tomhoule@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:parity/helix Feedback for Helix parity features cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Helix: v selects one line too many after d

4 participants