feat(vi): support deleting chars backward with X

[sim590]

Summary

Add support for the X command in vi normal mode, which deletes the character before the cursor and places it in the cut buffer, mirroring the behavior of X in vim.

This partially addresses #621.

Changes

• Parse X as Command::DeleteCharBackward in the vi command parser
• In normal mode, emit MoveLeft + CutChar to delete the character before the cursor and place it in the cut buffer
• In visual mode, emit CutSelection (consistent with x behavior)
• Add parser tests for X and 2X

Known limitation

When used with a multiplier (e.g. 3X), each character is cut individually, so only the last deleted character remains in the cut buffer. This is a pre-existing limitation shared with x and will be addressed in a follow-up PR.

[kronberger-droid]

Parser side looks right.

One correctness issue: MoveLeft + CutChar doesn't guard against the cursor not moving. So we have two cases where it diverges from Vim, at pos 0 it cuts the char on cursor and on end of a line in multi-line it would cut \n and merge the lines.

EditCommand::Backspace already gets this right via line_buffer::delete_left_grapheme. Could you add an EditCommand::CutCharLeft mirroring CutChar but with that goard and map DeleteCharBackward to it?

Two editor-level tests would be also good, for pos 0 in single line and start of line 2 in multi-line.

If you got that I would be fine to land it.

[sim590]

OK. I got that.

Now another thing I just realized with my current implementation is that X in visual selection should rather cut the whole lines that are covered by the selection. So, a simple Editor::cut_selection_to_cut_buffer is not correct. The proper behaviour should be to get the range of the selection and extend both the lower and upper bounds to respectively the begening of the first line and the end of the last line in the selection range.

Do you agree with this addition?

[sim590]

@kronberger-droid: I did the addition for visual selection behaviour for X in b472620 and 68616b2. The approach is to generalize code of current_line_range into line_range_for(usize) which returns the line range for the position passed as argument. Then, the visual selection is easily extended to the full lines. It works correctly. I put up three more tests for that.

[kronberger-droid]

Hmm do you think you have to define a new edit command to achieve the guard?
Before you built the CutCharLeft in the mode out of primitives, now you have defined a new one. I would love if you could try to achieve the same, but with the primitives.

Thanks for the effort!

[sim590]

The issue is that neither MoveLeft nor Backspace currently guard against crossing newlines. They both use grapheme_left_index() unconditionally. So composing MoveLeft + CutChar doesn't work at line boundaries.

I see a few options:

1. Add a current_line: bool parameter to MoveLeft (like MoveRightUntil already has) and use MoveLeft { select: false, current_line: true } + CutChar. This would not introduce any regression since the default would remain false.
2. Modify Backspace to copy into the cut buffer. However, this would regress insert mode and emacs mode, where backspace should not fill the cut buffer. Since editor.rs is mode-agnostic, there's no way to conditionally enable this.
3. Keep CutCharLeft as a dedicated command. This mirrors the existing Delete/CutChar pattern: Delete removes without copying, CutChar removes and copies to the cut buffer. Similarly, Backspace removes left without copying, CutCharLeft removes left and copies. It also naturally handles the visual-mode linewise cut behavior.

Which approach would you prefer?

[kronberger-droid]

Thanks for the rundown.
It seems move left should actually guard for newline.
There are two different ways a newline can occur. Either you are in single line mode, where just move left and cut would be fine, since it just wraps the text and is essentially one line.
And there is multi line mode where if we want to provide a vim like experience it has to guard against newlines anyways.
And those states should be available in the editor.

Also I think if you want to make 3X work you will have to use something like option 1 or making MoveLeft guarding anyways right?

I am now working on an idea for much stronger primitives in EditCommand which will make these types of problems much easier to handle. But until then for me its just important to be sure about the intent of an EditCommand so its easy to replace them if we have stronger primitives. Until then I am fine with what works best for count prefixes (3X etc.)

Still important to have a more complete version as a baseline. Thanks!

[sim590]

Based on your feedback, I'll switch to option 1: adding current_line: bool to MoveLeft and composing MoveLeft { select: false, current_line: true } + CutChar for normal mode. This removes the need for CutCharLeft.

Note that 3X will only keep the last deleted character in the cut buffer, but this is a pre-existing limitation shared with 3x (apply_multiplier repeats CutChar N times, each overwriting the register). I'd rather fix both together in a follow-up PR.

The remaining open question is visual mode: X in Vim cuts entire lines covered by the selection (linewise), not just the selection itself. This vi-specific behavior needs access to the editor state to extend the selection to line boundaries, but command.rs only returns Vec<ReedlineOption>. Would you prefer I add a CutSelectionLinewise primitive for now, or leave visual mode linewise behavior for a follow-up?

[kronberger-droid]

Hmm I would avoid a view of the editor in the modes. (against what I mentioned in #960)

If you want you can propose a followup to fix the register behavior for multiple actions, but I a can't promise I will merge it, since it seems without some better primitives this will be just hacky.
But maybe you find a beautiful solution.

It would be nice to still add tests for the expected behavior we can't implement right now and skip them.
This would be a valueable followup.
Thanks!

[sim590]

Just to clarify on the visual mode question: since command.rs would only emit an EditCommand without needing access to editor state, would adding a CutSelectionLinewise primitive be acceptable? It would live entirely in editor.rs (extending the selection to line boundaries and cutting), and the vi module would just emit it the same way it currently emits CutSelection. Or would you prefer to leave visual mode linewise behavior for a follow-up?

EDIT: This is irrelevant if we keep CutCharLeft.

[sim590]

After trying the implementation for MoveLeft + CutChar, I came full circle and realized what you said at first: it cannot be done that way because of the beginning of the line edge case where CutChar would cut characters even though the cursor didn't move. So, in the end, we need to keep the current implementation.

[kronberger-droid]

After trying the implementation for MoveLeft + CutChar, I came full circle and realized what you said at first: it cannot be done that way because of the beginning of the line edge case where CutChar would cut characters even though the cursor didn't move. So, in the end, we need to keep the current implementation.

Do I understand that correctly that you would then stay with this implementation? Just tell me when I should take a closer look.

[sim590]

Yes, that is correct. I think that until the better primitives you are working on are integrated, we cannot do without a new primitive CutCharLeft in order to correctly handle the beginning of line edge cases. Therefore, the current implementation is the correct one, I believe. You can take a closer look.