Add Helix editing mode

[savente93]

Hi folks 👋

I'm opening up a draft PR even though it's not anywhere near done to get some early feedback as it's quite a big change. If this eventually lands it should fix #639 . Very open to feedback from others, but currenly my plan of action is broadly:

• duplicate vi mode and rename it to hx mode
• flip grammar structure
• adjust keybindings to use what helix has
• implement selection highlighting (optional)
• cleanup/polish

I'm putting the selection highlighting as optional even though it's actually quite a core component of the helix model (it's actually why the grammar got flipped, so you can see what you're making adjustments on) mostly because I have no idea how attanable it is, and small steps and all that.

Looking forward to you feedback!

[fdncred]

I know there have been quite a few people ask about helix mode, but this is quite ambitious. You might want to start with the founding principle of helix which is selection and just create a PR that allows one to do selection with keybindings. e.g. switch into selection mode, use left, right arrow keys to start and stop selection, switch out of selection mode, or something like that.

Once we have a selection mode, it seems like this PR is possible, but not really until that. Just my opinion though.

[savente93]

I think that is a good point actually. I'm barely a hop, skip and a jump into the implementation and it's already becoming quite clear that it makes very little sense to work on this without having some kind of selection mechanism in place. The one thing I'm not quite sure of yet, is how these selections may or may not interact with Vi and/or emacs mode.

edit: sorry didn't read the comment carefully enough on the first pass. just noticed the mention of a selection mode

[sholderbach]

Thank you for immediately giving this a shot and laying out a strategy to approach that. I really appreciate that.

I want to be honest, why I have some concerns about landing this feature right now and why I think there should be a pretty high bar for an additional edit mode.

While our "emacs"-like mode is pretty flexible and open for configuration the existing "vi"-mode is pretty hacky through reusing the same operations internally.
It has thus not only missing features (e.g. glaringly no visual mode, missing text objects) but also major deviations from expected behavior (read bugs). Some of them could be overcome in the existing framework by tweaking those EditCommands but for others the complexity cost would be lower if we rework some of those things in terms of ranges/spans/selections.
A good example for the former is #631 (where I owe @alsuren a test/review cycle). When we make those changes for/from the established edit modes this will either cost you some rebases in this branch or all other contributors new things to watch out for in the helix mode as soon as it lands.

In the context that we currently both have some severe deficit in end to end UI tests that can reliably catch complex regressions and are understaffed in terms of technical maintainers steeped in reedline I don't want to disappoint expectations/ambitions.
The primary goal for reedline in the context of Nushell aiming for 1.0 will be to become reliable in behavior and settled in terms of existing configuration points. Under those constraints, giving more attention to the modes that have orders of magnitude more users or contributors familiar with them is obvious.
Nevertheless if this effort gets us incrementally closer to a simpler more maintainable and reliable core I don't want to rule this out (despite my bias of having no experience with the kakoune or helix flavored bindings)

[savente93]

So to make sure I've understood you correctly i'm going to try and summeraise your main points (feel free to point out any missunderstandings)

1. editing modes like vi-mode have deficiencies that need attention
2. vim has orders of magitude more users, therfore it makes sense to tempt them over by improving vi mode
3. the UI side is understaffed both in terms of infra and maintainers so we don't want to add more complexities

The conclusion you didn't mention explicity but I deduce from this is "we should not be doing this right now".
(this is the jist of what I understand)

Assuming I understood correctly some responses below, but tl;dr is: that's valid, I agree and if not now, then when?

So firstly, you say "landing this feature right now" (no sarcasm intended) but this feature is not, nor do I expect it to be any time soon, anywhere near ready. I definitely agree that the bar for entry should be high. A crappy implementation would hurt both Nu and Helix (imo). So I did not expect to land this anytime soon, or without any discussion, I kinda just jumped in head first. As an extention of that, discussions like these are exactly why I opened a PR before doing any significant work, so I'm glad we're having this discussion.

Secondly, as @fdncred mentioned and I recently found out, it really doesn't make sense to try and implement any of the helix bindings without having a mechanism for selections. This is something you mentioned could also benifit emacs and/or vi mode so working on that first would be good for everyone.

Lastly, as to the fate of this PR. I think we all seem to be in agreement that it doesn't make sense to continue this at this moment, but I do have hope for a helix mode some day. I can leave it open as I do still think the approach is valid, even if it is currently blocked. I can see arguments both for leaving it open, or closing it and opening a new one when the time is right. I'm happy to defer to others on that front. Though I will also say, that if we're going to close this, I hope we can come to at least some agreed criteria for when it makes sense to open it back up, but stuff is hard so I fully accept the answer to that might be 🤷‍♀️

Thoughts?

[sholderbach]

I think what I would gather as a reasonable point where it makes sense to go ahead with helix mode without impacting the effort to getting the rest of the modes nice would be:

• We have maybe reduced the number of EditCommand implementations that do essentially the same in slight variations: Move/Cut/Delete:

  reedline/src/core_editor/editor.rs

  Lines 43 to 98 in b1344f6

  	EditCommand::MoveToStart => self.line_buffer.move_to_start(),
  	EditCommand::MoveToLineStart => self.line_buffer.move_to_line_start(),
  	EditCommand::MoveToEnd => self.line_buffer.move_to_end(),
  	EditCommand::MoveToLineEnd => self.line_buffer.move_to_line_end(),
  	EditCommand::MoveToPosition(pos) => self.line_buffer.set_insertion_point(*pos),
  	EditCommand::MoveLeft => self.line_buffer.move_left(),
  	EditCommand::MoveRight => self.line_buffer.move_right(),
  	EditCommand::MoveWordLeft => self.line_buffer.move_word_left(),
  	EditCommand::MoveBigWordLeft => self.line_buffer.move_big_word_left(),
  	EditCommand::MoveWordRight => self.line_buffer.move_word_right(),
  	EditCommand::MoveWordRightStart => self.line_buffer.move_word_right_start(),
  	EditCommand::MoveBigWordRightStart => self.line_buffer.move_big_word_right_start(),
  	EditCommand::MoveWordRightEnd => self.line_buffer.move_word_right_end(),
  	EditCommand::MoveBigWordRightEnd => self.line_buffer.move_big_word_right_end(),
  	EditCommand::InsertChar(c) => self.line_buffer.insert_char(*c),
  	EditCommand::Complete => {}
  	EditCommand::InsertString(str) => self.line_buffer.insert_str(str),
  	EditCommand::InsertNewline => self.line_buffer.insert_newline(),
  	EditCommand::ReplaceChar(chr) => self.replace_char(*chr),
  	EditCommand::ReplaceChars(n_chars, str) => self.replace_chars(*n_chars, str),
  	EditCommand::Backspace => self.line_buffer.delete_left_grapheme(),
  	EditCommand::Delete => self.line_buffer.delete_right_grapheme(),
  	EditCommand::CutChar => self.cut_char(),
  	EditCommand::BackspaceWord => self.line_buffer.delete_word_left(),
  	EditCommand::DeleteWord => self.line_buffer.delete_word_right(),
  	EditCommand::Clear => self.line_buffer.clear(),
  	EditCommand::ClearToLineEnd => self.line_buffer.clear_to_line_end(),
  	EditCommand::CutCurrentLine => self.cut_current_line(),
  	EditCommand::CutFromStart => self.cut_from_start(),
  	EditCommand::CutFromLineStart => self.cut_from_line_start(),
  	EditCommand::CutToEnd => self.cut_from_end(),
  	EditCommand::CutToLineEnd => self.cut_to_line_end(),
  	EditCommand::CutWordLeft => self.cut_word_left(),
  	EditCommand::CutBigWordLeft => self.cut_big_word_left(),
  	EditCommand::CutWordRight => self.cut_word_right(),
  	EditCommand::CutBigWordRight => self.cut_big_word_right(),
  	EditCommand::CutWordRightToNext => self.cut_word_right_to_next(),
  	EditCommand::CutBigWordRightToNext => self.cut_big_word_right_to_next(),
  	EditCommand::PasteCutBufferBefore => self.insert_cut_buffer_before(),
  	EditCommand::PasteCutBufferAfter => self.insert_cut_buffer_after(),
  	EditCommand::UppercaseWord => self.line_buffer.uppercase_word(),
  	EditCommand::LowercaseWord => self.line_buffer.lowercase_word(),
  	EditCommand::SwitchcaseChar => self.line_buffer.switchcase_char(),
  	EditCommand::CapitalizeChar => self.line_buffer.capitalize_char(),
  	EditCommand::SwapWords => self.line_buffer.swap_words(),
  	EditCommand::SwapGraphemes => self.line_buffer.swap_graphemes(),
  	EditCommand::Undo => self.undo(),
  	EditCommand::Redo => self.redo(),
  	EditCommand::CutRightUntil(c) => self.cut_right_until_char(*c, false, true),
  	EditCommand::CutRightBefore(c) => self.cut_right_until_char(*c, true, true),
  	EditCommand::MoveRightUntil(c) => self.move_right_until_char(*c, false, true),
  	EditCommand::MoveRightBefore(c) => self.move_right_until_char(*c, true, true),
  	EditCommand::CutLeftUntil(c) => self.cut_left_until_char(*c, false, true),
  	EditCommand::CutLeftBefore(c) => self.cut_left_until_char(*c, true, true),
  	EditCommand::MoveLeftUntil(c) => self.move_left_until_char(*c, false, true),
  	EditCommand::MoveLeftBefore(c) => self.move_left_until_char(*c, true, true),

• We have a selection implementation in at least one of the existing modes (be it visual mode for vim or Shift-Arrow for the non modal emacs-like mode)
• We are happy with how you configure movements that can affect both the buffer and the general UI (e.g. up/down for history, completion or other menus) We currently have this UntilFound logic that has some implicit rules in how you need to order things and has some edge cases (multiplier motions in vi, left and right movements while in the history scrollback). This is not super critical for the helix implementation concerns itself but I think it makes sense that we have some clarity in how things should be configured on the nushell side so we can drop helix mode in when it is ready (potentially after we stabilize a config format for 1.0)

[savente93]

Sounds like a good conclusion to me, and it seem like we're on the same page. Given that you didn't express a preference for either closing the PR or keeping it I think I'll be closing it to avoid having to do rebase work down the line. Once the time is there it makes more sense to revaluate if the approach even makes sense. Thanks for the good discussion!

[sholderbach]

Thank you for being proactive here!