Vi mode text objects for word, WORD, brackets and quotes#939
Conversation
…tespace Note a buffer full of whitespace is not properly considered and still causing incorrect behaviour. TODO fix this.
…add general yank/cut range methods
…eatures - Now handles jumps to next open/close or equal symbol pairs if not in a pair already - Searching only on current line for equal symbol pairs e.g. quotes - Correctly handles graphemes
- Refactor the structure of the methods to get ranges, don't need to pass in depth unecessarily, high level functions don't require cursor passed in. - Now two seperate functions for ranges, one "next" and one "current" range that gets you either the range inside next text object or inside current one depending on position of cursor. - Finilise logic to correctly handle graphemes (not byte sized chars) TODO Update unit tests
- Improve some text object ranges to use iterators rather than complex logic - Clean up documentation, add consts etc - Look through and refactor some editor functions
… and rewrite a lot of doc strings
ayax79
left a comment
There was a problem hiding this comment.
I have been wanting this for a while. This much better than the hack a while back.
The code looks clean and seems to work fine in nushell (minus a couple minor changes).
Great work!
@ayax79 Glad to help! These motions are so hard coded into my brain it felt clunky trying to edit without them haha. |
| EditCommand::CopyInsidePair { .. } => EditType::EditText, | ||
| EditCommand::CutAroundPair { .. } => EditType::EditText, | ||
| EditCommand::CopyAroundPair { .. } => EditType::EditText, |
There was a problem hiding this comment.
I believe the EditType for Copy command should be a noop.
| EditCommand::CopyInsidePair { .. } => EditType::EditText, | |
| EditCommand::CutAroundPair { .. } => EditType::EditText, | |
| EditCommand::CopyAroundPair { .. } => EditType::EditText, | |
| EditCommand::CopyInsidePair { .. } => EditType::NoOp, | |
| EditCommand::CutAroundPair { .. } => EditType::EditText, | |
| EditCommand::CopyAroundPair { .. } => EditType::NoOp, |
|
Let's move forward with this. Thanks! |
|
@JonLD would you mind looking at this PR nushell/nushell#16791. I left out https://github.com/nushell/reedline/pull/939/files#diff-2dbf826b03a6d3f8d1d1e31324494945d307b79a9592d7eddf70f1d9bdfd1473R408-R416 because i wasn't sure how to implement them with TextObject in nushell. |
|
Thanks for getting this merged! As mentioned on the MR, I believe the text object types need to be publicly exposed so the correct EditCommand can be called. |
This PR updates nushell to the latest reedline commit 32e82c27. Notes from @JonLD regarding reedline's new `TextObject`, reedline change author of nushell/reedline#939 and nushell/reedline#957 and #16808 Since EditCommand contains a TextObject and not a string it is not possible for the user to directly there needs to be a bit of work done to construct the relevant text object. Maintains the edit command names, seen under "keybindings list", of CutTextObject and CopyTextObject and using scope: <inner|around> and the object_type: mirrors the Reedline implementation and the informative error messages should help signal to users what are valid inputs. Ideally the "keybindings list" would be a bit mor informative as to how to set up specific keybindings but that's not currently possible, and perhaps not even necessary. The alternative would be to set up edit commands for each text object, this would be a larger Reedline change and add unecessary complexity to Reedline but would expose the commands better to the use. You could just add them in this file and keep Reedline as is but that would not be exposed to the user via "keybindings list" as this command lists the actual enums and not commands set up in the reedline_config.rs. ## Release notes summary - What our users need to know Breaking change since the reedline change renames `cutinside` and `yankinside`. The PR adds these new EditCommands: - cutinsidepair - copyinsidepair - cutaroundpair - copyaroundpair - cuttextobject - copytextobject ## Tasks after submitting N/A
* fix: prompt glitch when resizing with cursor in a multiline command (nushell#898) * fix: prompt glitch when resizing with cursor in a multiline command * fix: considering line wraps * fix: clippy * Bump version to `0.41.0` (nushell#936) * Protect against invalid suggestion spans (nushell#915) * Protect against invalid suggestion spans * Protect against start > end * feat: Builder option to immediately accept input (nushell#933) * feat: Option to immediately accept input * chore: PR feedback * feat: add `ViChangeMode` event (nushell#932) * feat: add `ViChangeMode` event * fix: use `FromStr` trait * fix: undo unused change * fix: clippy * Fix `mismatched_lifetime_syntaxes` (nushell#947) From Rust 1.89.0 on this lints. https://blog.rust-lang.org/2025/08/07/Rust-1.89.0/#mismatched-lifetime-syntaxes-lint * Fix missing import in README.md (nushell#942) Co-authored-by: sholderbach <sholderbach@users.noreply.github.com> * fix: dislocation of cursor after previous_history navigation to a multiline entry (nushell#899) * bumping version to 42 (nushell#949) Co-authored-by: Jack Wright <jack.wright@nike.com> * Bump `rusqlite` to 0.37 (nushell#950) * feat: make columnar menu traversal direction configurable (nushell#951) * Make columnar menu traversal direction configurable * Add tests for columnar menu selection position updates * Apply changes based on review * Upgrade GitHub Actions and Rust toolchain versions (nushell#954) * Upgrade GitHub Actions and Rust toolchain versions * Update .github/workflows/ci.yml * Vi mode text objects for word, WORD, brackets and quotes (nushell#939) * Addd initial change inner and around word text objects and handle whitespace Note a buffer full of whitespace is not properly considered and still causing incorrect behaviour. TODO fix this. * Renaming functions and fix default value of current_whitespace_range_start * Rename cut/yank inside enums and methods to cut/yank inside pair and add general yank/cut range methods * Use TextObject enum instead of passing through the character * WIP: Add quote and bracket text objects and add jumping if not inside objects * Add my own methods for finding matching pair and jumping * Fix bugs in new function to get matching pair range and it's finish features - Now handles jumps to next open/close or equal symbol pairs if not in a pair already - Searching only on current line for equal symbol pairs e.g. quotes - Correctly handles graphemes * Simplify heirarchy of pair range finding functions - Refactor the structure of the methods to get ranges, don't need to pass in depth unecessarily, high level functions don't require cursor passed in. - Now two seperate functions for ranges, one "next" and one "current" range that gets you either the range inside next text object or inside current one depending on position of cursor. - Finilise logic to correctly handle graphemes (not byte sized chars) TODO Update unit tests * Refactoring range functions and tidy up/extend unit test cases and coverage * More refactoring - Improve some text object ranges to use iterators rather than complex logic - Clean up documentation, add consts etc - Look through and refactor some editor functions * Move text object range methods into line_buffer from editor * Combine line_buffer quote and pair text object functions into generic and rewrite a lot of doc strings * Testing for quote and bracket text object functions in editor.rs * Whitespace * Rework unit tests for new function structure * Remove angle brackets from b text object * Rename yank text object functions to copy * Add bracket test cases to range_inside_next_pair_in_group unit tests * Add more detailed unicode safety tests * Fix display enum string for renamed enums * Unicode and overflow/underflow safety when expanding text object ranges * Pass through matching pair group const for quote and bracket text object functions * Rename yank_range -> copy_range for consistency with other methods * Remove unecessary guard clause from expand_range_to_include_pair * Correct display string for CutInsidePair * Make text object types public (nushell#957) * Make TextObject, TextObjectScope and TextObjectType public * Correct CopyInsidePair and CopyAroundPair EditType to NoOp * fix: dislocation of cursor during history navigation (nushell#959) * fix: dislocation of cursor during history navigation * test: new test case by Claude Sonnet * simplify * Fix typos (nushell#962) * Bump version for `0.43.0` release (nushell#961) * Fix shift selection in vi (insert) & emacs mode (nushell#927) * Add match_indices field to Suggestion (nushell#798) * Add match_indices field to Suggestion Make columnar_menu use match indices Make ide menu use match indices Add fuzzy completions example Test style_suggestion Make doctests in default.rs pass Highlight entire graphemes Extract ANSI escapes from strings to apply match highlighting Fix clippy lint for fuzzy completion example Shut the typo checker up Use existing variable `escape` Copy regex from parse-ansi crate * replace LazyLock with lazy_static that works with Rust 1.63.0 (#2) * Homegrown ANSI parser Fix padding for columnar menu Highlight substring matches too by default Simplify (?) columnar menu * Fix clippy lints after rebase * Use get_match_indices helper * Stop using 'fo' because it's a typo? Fo shizzle. * Use to_string() instead of as_str() * Style entire suggestion same color * RESET after suggestion --------- Co-authored-by: Divanshu Grover <divanshugrover2009@gmail.com> * fix bashism parsing (nushell#958) * refactor: use crossterm `supports_keyboard_enhancement` once when KittyProtocolGuard is initialized (nushell#920) Co-authored-by: Pierre POLLET <bouchon.sappiness682@passinbox.com> --------- Co-authored-by: zc he <blindfs19@gmail.com> Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com> Co-authored-by: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Co-authored-by: Stuart Carnie <stuart.carnie@gmail.com> Co-authored-by: Daniel Bonofiglio <63377579+bonofiglio@users.noreply.github.com> Co-authored-by: Daniel del Castillo <52696601+CastilloDel@users.noreply.github.com> Co-authored-by: Jack Wright <56345+ayax79@users.noreply.github.com> Co-authored-by: Jack Wright <jack.wright@nike.com> Co-authored-by: Piepmatz <git+github@cptpiepmatz.de> Co-authored-by: simonborje <simonborje@gmail.com> Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> Co-authored-by: JonLD <73548328+JonLD@users.noreply.github.com> Co-authored-by: Collin Murch <collin@murch.net> Co-authored-by: Divanshu Grover <divanshugrover2009@gmail.com> Co-authored-by: migraine-user <mj9012159@gmail.com> Co-authored-by: PtiBouchon <31276594+PitiBouchon@users.noreply.github.com> Co-authored-by: Pierre POLLET <bouchon.sappiness682@passinbox.com>
* Addd initial change inner and around word text objects and handle whitespace Note a buffer full of whitespace is not properly considered and still causing incorrect behaviour. TODO fix this. * Renaming functions and fix default value of current_whitespace_range_start * Rename cut/yank inside enums and methods to cut/yank inside pair and add general yank/cut range methods * Use TextObject enum instead of passing through the character * WIP: Add quote and bracket text objects and add jumping if not inside objects * Add my own methods for finding matching pair and jumping * Fix bugs in new function to get matching pair range and it's finish features - Now handles jumps to next open/close or equal symbol pairs if not in a pair already - Searching only on current line for equal symbol pairs e.g. quotes - Correctly handles graphemes * Simplify heirarchy of pair range finding functions - Refactor the structure of the methods to get ranges, don't need to pass in depth unecessarily, high level functions don't require cursor passed in. - Now two seperate functions for ranges, one "next" and one "current" range that gets you either the range inside next text object or inside current one depending on position of cursor. - Finilise logic to correctly handle graphemes (not byte sized chars) TODO Update unit tests * Refactoring range functions and tidy up/extend unit test cases and coverage * More refactoring - Improve some text object ranges to use iterators rather than complex logic - Clean up documentation, add consts etc - Look through and refactor some editor functions * Move text object range methods into line_buffer from editor * Combine line_buffer quote and pair text object functions into generic and rewrite a lot of doc strings * Testing for quote and bracket text object functions in editor.rs * Whitespace * Rework unit tests for new function structure * Remove angle brackets from b text object * Rename yank text object functions to copy * Add bracket test cases to range_inside_next_pair_in_group unit tests * Add more detailed unicode safety tests * Fix display enum string for renamed enums * Unicode and overflow/underflow safety when expanding text object ranges * Pass through matching pair group const for quote and bracket text object functions * Rename yank_range -> copy_range for consistency with other methods * Remove unecessary guard clause from expand_range_to_include_pair * Correct display string for CutInsidePair
Overview
Implements the vim style inner and around (
ianda) text objects for:w(a sequence of letters, digits and underscores, separated with white space)W(a sequence of non-blank characters, separated with white space)b(any of( ),[ ],{ })q(any of" ",' ',``)di(orci") extended for around versions that cover the pair charactersThis addresses most of #848 although it doesn't add paragraph text object, that would be fairly straightforward to add later.
Additionally, the pair motions will now jump to the next pair if non are found within search range. For symmetric pairs like quotes searching is restricted to the current line. For asymmetric pairs like brackets search is multi-line across the whole buffer. Symmetric pairs searching does not do any parsing or matching of pairs based on language/groupings. It simply searches for the previous and next matching characters (this is consistently with vim but could be improved, perhaps using tree sitter).
Repeats
While it might not be the intend to achieve identical behaviour as vim I thought it useful to compare to the default vim behaviour of repeats.
Around brackets:
In the current implementation repeat motions for
aroundtext objects will cause a jump to the next bracket.e.g. 2daw on "(first)between(second)" while in the first bracket will delete both the first and second bracket, included the parentheses themselves, and move the cursor to the end "between".
Vim works around this by not letting you do repeats for "around" motions unless inside a nested bracket.
Inner words:
Repeat motions for words also differ from default vim behaviour for repeated word text objects. In vim a
2diwperforms a single around diw command and then the repeat command isdw. The difference in behaviour is that in the reedline implementation2dawondelete th|is wordwill result indelete|wordwhereas in vim it would bedelete |word.This could perhaps be worked around by adding a method of passing an alternate command for repeats to the command module which could be made in a future change. I think the current behaviour is satisfactory for most use cases.
Word identification
Currently due to using the unicode-segmentation crate for splitting at word word boundaries word does not have the same meaning as Vi/Vim. This means
diwonnot.a..wo|rdwill delete the whole WORD rather than just the word under the cursor.