Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Vim like movement commands #4204

Closed

Conversation

mitsuhiko
Copy link
Contributor

@mitsuhiko mitsuhiko commented Oct 11, 2022

This adds a few new movement commands to allow hjkl to be rebound so that they follow more the movement of vim. left/right movement cannot move between lines and up/down either follows newline or does not accidentally move onto the newline character. See #4268 for motivation. This however does not emulate vim, it just has some vim inspired movements. In particular it retains the entire behavior of Helix with regards to how newlines are otherwise handled (they stay selectable, and these alternative movement commands to not interfere with other commands in use or change their assumptions).

vim-movement mov

The following commands are added:

  • move_line_up_anchored (bind to k): implements anchored movement up
  • move_line_down_anchored (bind to j): implements anchored movement down
  • move_char_left_same_line (bind to h): implements same-line movement left
  • move_char_right_same_line (bind to l): implements same-line movement right
  • extend_line_up_anchored (bind to k): implements anchored selection extension up
  • extend_line_down_anchored (bind to j): implements anchored selection extension down
  • extend_char_left_same_line (bind to h): implements same-line selection extension left
  • extend_char_right_same_line (bind to l): implements same-line selection extension right

Bindings

[keys.normal]
j = ["move_line_down_anchored"]
k = ["move_line_up_anchored"]
h = ["move_char_left_same_line"]
l = ["move_char_right_same_line"]

[keys.select]
j = ["extend_line_down_anchored"]
k = ["extend_line_up_anchored"]
h = ["extend_char_left_same_line"]
l = ["extend_char_right_same_line"]

Same Line Movement

The default movement behavior of helix will happily place you in the next line. Vim users are not used to that and are likely to prefer the cursor not to move into the next line (that is at least my expectation). This is what move_char_left_same_line and move_char_right_same_line do. They will however let you move to the rightmost character (the newline) which is not what vim does (except of virtualedit=oneline is used). However this pairs up nicely with the anchored movement (see next).

Anchored Vertical Movements

The two commands move_line_up_anchored and move_line_down_anchored implement what I would call anchored movement. If you are on the newline character (eg: one to the right of where goto_line_end would place you) then as you move up and down you end up on the newline of the target line again. This emulates the behavior of vim of what happens if you move to the line end with $. If however you are placed to the left of the newline, as you move up and down you will not move onto the newline.

The main difference is that in vim if you use $ and go up/down you stay anchored to the line end. In Helix with this change you need to do gll to stay anchored to the newline. I do however have to say that I prefer this actually because in Vim you do not actually know if you are newline anchored visually. That is as far as I can tell an internal state.

I played around with this a bit now and I really like the combination of the two. It stays true to the helix character model, but it also feels very natural for vim users.

Limitation: Movements across empty lines are ambiguous. To help with this I put the horizontal column to the largest possible integer on the range to tell these cases apart. From what I can tell this does not create negative impact and avoids extra complication on the range.

Notes on Mixed Use

I think these move/extend commands are interesting to use independent of each other. For instance I find the anchored newline movement functionality useful even if one does not like restricting the movements within a line. For instance line based selection works really nice as once the visual selection is on the newline, any further moving down will continue to capture entire lines. That means for instance vx will continue to move downwards which I think also addresses some points of #1638 as it basically keeps you in line mode for as long as you don't move left.

This adds new movement commands to allow h/l to be rebound so that they
only operate within a single line like Vim does.  This however cannot be
fully emulated in Helix as Helix will happily place the character on the
newline rather than "before" like Vim does.  This means that if one
moves to the right, "i" is needed to insert rather than "a" what a Vim
user would expect.  "a" would still put the character into the new line.

The "move_from_line_end" command is provided which can optionally be
used after all movements to move the character one back if needed.

Unfortunately this also means that the cursor is repositioned which
causes it to forget the original position it had which makes movements
between lines annoying as the cursor has a left drift.
@mitsuhiko mitsuhiko force-pushed the feature/vim-like-line-movement branch from f01e27b to 205e3a1 Compare October 14, 2022 14:11
@mitsuhiko mitsuhiko marked this pull request as ready for review October 14, 2022 14:14
@mitsuhiko mitsuhiko force-pushed the feature/vim-like-line-movement branch from bc36983 to 0af9776 Compare October 14, 2022 16:49
@the-mikedavis
Copy link
Member

Newlines being selectable like any other character is intended behavior and I don't like adding options or alternate movement commands to skirt them. See #2956 and #3076.

@mitsuhiko
Copy link
Contributor Author

mitsuhiko commented Oct 14, 2022

@the-mikedavis this does not change the behavior about select-ability. They are still selectable as before. The new commands add a way that for horizontal movements you can stay constrained within the line but you still move onto them and for vertical movement it even anchors you onto them if you already are on one.

Just to be clear since maybe my comment on the other issue was misleading: i think the visible newlines are a good thing and I wouldn't want to change this. This is also not changing the behavior of helix to bring in vim semantics that do not feel native. The movements here are vim inspired, but they are actually following the helix model. Rather than using hidden internal flags for the newline behavior, this actually takes advantage of the visible and selectable newline instead.

@kirawi kirawi added A-helix-term Area: Helix term improvements A-core Area: Helix core improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 15, 2022
@archseer
Copy link
Member

archseer commented Nov 1, 2022

(Also fixes #768)

@mitsuhiko
Copy link
Contributor Author

Curious about what the odds are of this merging. I'm happy to incorporate necessary changes if there is something I can do here.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Looking at your comment here (#4204 (comment)), Vim whichwrap and the context in #768, I think these motions are ok and don't introduce incompatible Vim-isms 👍. (Sorry for jumping the gun on "no Vim" here; steam preemptively comes out of my ears when I see "Add Vim-like..." 😅)

helix-core/src/movement.rs Outdated Show resolved Hide resolved
helix-core/src/movement.rs Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis linked an issue Nov 28, 2022 that may be closed by this pull request
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
@mitsuhiko
Copy link
Contributor Author

An open question I have is how this should be documented best. Is there a place to document unmapped commands that do not have a typable alias?

@the-mikedavis
Copy link
Member

Not currently, see: #997

@mitsuhiko
Copy link
Contributor Author

Then I suppose this is ready.

@archseer
Copy link
Member

archseer commented Feb 2, 2023

Sorry for the delay, this looks fine to me but needs a rebase now that the soft wrap changes landed!

@pascalkuthe
Copy link
Member

The rebase will be slightly more challenging than usual because the vertical movement commands were essentially reimplemented in #5420. It would also be good to have a version that does visual line movement and document line movement like we have for the normal movement command

@mitsuhiko
Copy link
Contributor Author

I will update this. Unfortunately the changes are a bit more involved so it will take a bit until I find the time for it.

@goyalyashpal
Copy link
Contributor

i was also thinking about this topic the other day - that why don't n/vi/m stop the horizontal movement at line ends and don't wrap it around - is there any benefit to that way while editing etc...

@gabydd gabydd mentioned this pull request Mar 9, 2023
@mkblast
Copy link
Contributor

mkblast commented Apr 7, 2023

I think a better way to configure it is to add all of those binding into one option.

/// If you are not on the newline, then moving up and down will move you to the same
/// column except if that column is further to the left from where you are. In that case
/// it places you on the rightmost column that is not a newline.
pub fn move_vertically_anchored(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have some tests for both of these functions?

@pascalkuthe
Copy link
Member

I would definitely like to still see this feature. Unfortunately, this branch is quite stale as the refactor in #5420 entirely changed how movement works so somebody would need to start almost from scratch. Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a whichwrap option?
8 participants