Skip to content

feat(editor): Add helix match selection - #1188

Open
glcraft wants to merge 23 commits into
nushell:mainfrom
glcraft:main
Open

feat(editor): Add helix match selection#1188
glcraft wants to merge 23 commits into
nushell:mainfrom
glcraft:main

Conversation

@glcraft

@glcraft glcraft commented Aug 24, 2026

Copy link
Copy Markdown

Summary

Add match actions from helix to reedline, and bind the new behaviors to the helix mode.

Keybindings:

  • mi<TextObject> : select inside a text object
  • ma<TextObject> : select around a text object
  • ms<TextObject> : add the specified text object around the selection
  • md<TextObject> : remove the nearest specified text object around the selection
  • mr<old TextObject><new TextObject> : replace the nearest specified text object around the cursor with another text object

This includes the following new EditCommand:

  • SelectTextObject
  • AddTextObject
  • RemoveTextObject
  • ReplaceTextObject

Moreover, brackets and quotes are more granular in TextObjectType by differentiating each text object.

Additional notes

Redundant code has been removed, replaced by existing or new functions. In a nutshell, <Cut/Copy><Inside/Around>Pair had been replaced by <Cut/Copy>TextObject.

Detail here

Among that, the following EditCommand has been replaced:

  • EditCommand::CutInsidePair{start: char, end: char) => EditCommand::CutTextObject{scope: TextObjectScope::Inner, object_type: TextObjectType::XXX}
  • EditCommand::CutAroundPair{start: char, end: char) => EditCommand::CutTextObject{scope: TextObjectScope::Around, object_type: TextObjectType::XXX}
  • EditCommand::CopyInsidePair{start: char, end: char) => EditCommand::CopyTextObject{scope: TextObjectScope::Inner, object_type: TextObjectType::XXX}
  • EditCommand::CopyAroundPair{start: char, end: char) => EditCommand::CopyTextObject{scope: TextObjectScope::Around, object_type: TextObjectType::XXX}

In core_editor::editor::Editor, the following functions have been replaced:

  • cut_inside_pair => cut_text_object
  • cut_around_pair => cut_text_object
  • copy_inside_pair => copy_text_object
  • copy_around_pair => copy_text_object

Vi mode implementation has been updated towards that.

All tests passes.

@glcraft
glcraft marked this pull request as draft August 24, 2026 21:36
@fdncred

fdncred commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Hey @glcraft! Good to see you around. Thanks!

glcraft and others added 6 commits August 28, 2026 22:45
…1194)

The note asked for it to fold onto a char-class boundary predicate like
`Word` and `LongWord`. It cannot: keeping `can't` and `3.14` whole turns
on what flanks the `'` or the `.`, which is context rather than class, thus
UAX-29 segmentation stays its own scan.
Remove CutInsidePair, CutAroundPair, CopyInsidePair, CopyAroundPair and
replace them by CutTextObject, CopyTextObject.
Also clean the vi mode implementation to remove action inside/around
pair
@glcraft

glcraft commented Sep 1, 2026

Copy link
Copy Markdown
Author

I updated the first comment

@glcraft
glcraft marked this pull request as ready for review September 3, 2026 12:21

@kronberger-droid kronberger-droid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dropping CutInsidePair, CopyInsidePair, CutAroundPair and CopyAroundPair is fine, nushell can follow the rename in crates/nu-cli/src/reedline_config.rs:1325-1351.
What it cannot follow is the loss of arbitrary pairs: the old variants take any left/right chars, TextObjectType covers a fixed set, so CutInsidePair { left: '|', right: '|' } has no equivalent.
Add TextObjectType::Pair { left: char, right: char } taking any two chars. The helix keymap keeps its fixed set, config-built bindings go through the new variant.

Comment thread src/core_editor/editor.rs
} else {
cursor.anchor()
},
cursor.head() - 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The cursor arithmetic in remove_text_object is wrong in three cases.
A position should shift by 1 once it is at or past range.start, and by 2 once it is past range.end, since that is how many delimiters were removed before it.

  • cursor.head() - 1 is applied unconditionally, so it underflows when the head is 0.
    In helix: type (bar) x, Esc, v, gh, md( panics with attempt to subtract with overflow.
  • cursor.anchor() > range.start misses an anchor exactly on range.start.
    "(abc)" with selection (1, 4) gives "abc" with (1, 3), which no longer covers abc.
  • An anchor past the closing delimiter shifts by 1 instead of 2.
    "(ab) cd" with selection (6, 2) gives "ab cd" with (5, 1), where 'd' is at 4.

Comment thread src/core_editor/editor.rs
} else {
cursor.anchor()
},
cursor.head() - 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The cursor arithmetic in remove_text_object is wrong in three cases.
A position should shift by 1 once it is at or past range.start, and by 2 once it is past range.end, since that is how many delimiters were removed before it.

  • cursor.head() - 1 is applied unconditionally, so it underflows when the head is 0.
    In helix: type (bar) x, Esc, v, gh, md( panics with attempt to subtract with overflow.
  • cursor.anchor() > range.start misses an anchor exactly on range.start.
    "(abc)" with selection (1, 4) gives "abc" with (1, 3), which no longer covers abc.
  • An anchor past the closing delimiter shifts by 1 instead of 2.
    "(ab) cd" with selection (6, 2) gives "ab cd" with (5, 1), where 'd' is at 4.

Comment thread src/core_editor/editor.rs
}
let cursor = self.line_buffer.cursor();
self.line_buffer.set_cursor(Cursor::point(cursor.head()));
let Some(range) = self.text_object_range(TextObject {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

text_object_range falls back to the next pair when none surrounds the cursor. That suits mi/ma, not md/mr.
With the cursor in foo of "foo (bar)", md( gives "foo bar" and mr([ gives "foo [bar]"; helix leaves the line alone.
Look up only the surrounding pair here and in replace_text_object at line 1833.

Comment thread src/enums.rs

EditCommand::AddTextObject { .. }
| EditCommand::RemoveTextObject { .. }
| EditCommand::ReplaceTextObject { .. } => EditType::MoveCursor { select: true },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These three edit the buffer but are typed EditType::MoveCursor.
On main since #1208, a MoveCursor command that leaves the cursor unchanged reports Inapplicable and returns before the undo update. ReplaceTextObject restores the original cursor exactly, so mr edits the buffer, creates no undo point, and lets UntilFound run the next event as well.
Type them EditType::EditText, drop the undo special case in run_edit_command, and add them to its leaves_selection list.

count,
Verb::CollapsingMotion(MotionTarget::LineStartNonBlank),
Some(HelixMode::Insert),
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Vi loses di$, ci$, yi$, da$ and ya$: bracket_pair_for on main maps $ to a pair, TextObjectType::from_char has no $.
Nothing on main tests them, which is why the suite still passes.
Keep $ and add a parser test for it.

Comment thread src/core_editor/editor.rs
TextObjectType::Quotes(TextObjectQuote::SingleQuote) => ('\'', '\''),
TextObjectType::Quotes(TextObjectQuote::DoubleQuote) => ('"', '"'),
TextObjectType::Quotes(TextObjectQuote::Tick) => ('`', '`'),
_ => return,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

msw and msW pass complete_pending and then do nothing here; mdw and mrw do the same through the guards in remove_text_object and replace_text_object.
Reject Word and BigWord in complete_pending for s, d and r.

Comment thread src/enums.rs
DoubleQuote,
/// \`
Tick,
/// (, ), \[, ], {, }, <, >

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the quote enum, so ', " and `, not the brackets.

@kronberger-droid

Copy link
Copy Markdown
Collaborator

Still some things me or Claude found, especially the panic is a big problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants