feat(editor): Add helix match selection - #1188
Conversation
|
Hey @glcraft! Good to see you around. Thanks! |
It was just for me, I noticed something and had to issue it
…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
|
I updated the first comment |
kronberger-droid
left a comment
There was a problem hiding this comment.
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.
| } else { | ||
| cursor.anchor() | ||
| }, | ||
| cursor.head() - 1, |
There was a problem hiding this comment.
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() - 1is applied unconditionally, so it underflows when the head is 0.
In helix: type(bar) x,Esc,v,gh,md(panics withattempt to subtract with overflow.cursor.anchor() > range.startmisses an anchor exactly onrange.start.
"(abc)"with selection(1, 4)gives"abc"with(1, 3), which no longer coversabc.- 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.
| } else { | ||
| cursor.anchor() | ||
| }, | ||
| cursor.head() - 1, |
There was a problem hiding this comment.
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() - 1is applied unconditionally, so it underflows when the head is 0.
In helix: type(bar) x,Esc,v,gh,md(panics withattempt to subtract with overflow.cursor.anchor() > range.startmisses an anchor exactly onrange.start.
"(abc)"with selection(1, 4)gives"abc"with(1, 3), which no longer coversabc.- 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.
| } | ||
| let cursor = self.line_buffer.cursor(); | ||
| self.line_buffer.set_cursor(Cursor::point(cursor.head())); | ||
| let Some(range) = self.text_object_range(TextObject { |
There was a problem hiding this comment.
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.
|
|
||
| EditCommand::AddTextObject { .. } | ||
| | EditCommand::RemoveTextObject { .. } | ||
| | EditCommand::ReplaceTextObject { .. } => EditType::MoveCursor { select: true }, |
There was a problem hiding this comment.
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), | ||
| ), |
There was a problem hiding this comment.
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.
| TextObjectType::Quotes(TextObjectQuote::SingleQuote) => ('\'', '\''), | ||
| TextObjectType::Quotes(TextObjectQuote::DoubleQuote) => ('"', '"'), | ||
| TextObjectType::Quotes(TextObjectQuote::Tick) => ('`', '`'), | ||
| _ => return, |
There was a problem hiding this comment.
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.
| DoubleQuote, | ||
| /// \` | ||
| Tick, | ||
| /// (, ), \[, ], {, }, <, > |
There was a problem hiding this comment.
This is the quote enum, so ', " and `, not the brackets.
|
Still some things me or Claude found, especially the panic is a big problem. |
Summary
Add match actions from helix to reedline, and bind the new behaviors to the helix mode.
Keybindings:
mi<TextObject>: select inside a text objectma<TextObject>: select around a text objectms<TextObject>: add the specified text object around the selectionmd<TextObject>: remove the nearest specified text object around the selectionmr<old TextObject><new TextObject>: replace the nearest specified text object around the cursor with another text objectThis includes the following new EditCommand:
SelectTextObjectAddTextObjectRemoveTextObjectReplaceTextObjectMoreover, 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>Pairhad 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_objectcut_around_pair=>cut_text_objectcopy_inside_pair=>copy_text_objectcopy_around_pair=>copy_text_objectVi mode implementation has been updated towards that.
All tests passes.