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

Fix cursor position bugs related to o and O #281

Merged
merged 2 commits into from
Jun 17, 2021

Conversation

sudormrfbin
Copy link
Member

  • O at the beginning of file didn't move cursor
  • o and O messed up cursor position with multiple cursors

Fixes #127

- `O` at the beginning of file didn't move cursor
- `o` and `O` messed up cursor position with multiple cursors

Fixes helix-editor#127
Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

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

Nice, I was trying this out the other day but I didn't know you need to do change_by_selection. Seemed bad that we need to keep a state around. The bug is fixed but the behavior seemed to be different from kakoune and vim. 10Oa now does not write 10 a but instead just one a 10 lines above, I think this is a bit surprising.

@sudormrfbin
Copy link
Member Author

Nice, I was trying this out the other day but I didn't know you need to do change_by_selection

The problem doesn't stem from using change instead of change_by_selection though, the issue is with properly calculating how much an edit needs to be offset by prior edits in the same transaction. As @archseer said on Matrix it'd be cleaner of we could handle this in change_by_selection itself.

10Oa now does not write 10 a but instead just one a 10 lines above, I think this is a bit surprising.

It's deviant from vim and kakoune yeah but do we really need to keep that behavior in helix ? It feels at odds with the multiple cursor philosophy (this commit didn't change this behavior though, it previously did not write 10 a's either).

@archseer
Copy link
Member

10Oa now does not write 10 a but instead just one a 10 lines above, I think this is a bit surprising.

This was already a problem before this PR, the implementation simply doesn't do that yet.

do we really need to keep that behavior in helix ?

I do want this, 10o should make 10 lines and place a cursor on each. It's an easy way to start writing multiple lines with multiple cursors.

@sudormrfbin
Copy link
Member Author

How should counts before i, I, etc be handled ? In vim 10i<text>Esc inserts <text> on the same line 10 times, which isn't very useful. IMO something more useful would be putting cursors on the current line and the next 9 lines in the same column. Same for 10I but in the beginning of each line.

@archseer
Copy link
Member

I think it's OK if counts on i are just ignored.

Do you want to do the change for 10o here, or should that be a follow-up?

@sudormrfbin
Copy link
Member Author

sudormrfbin commented Jun 17, 2021

I'll add it as a separate commit in this PR.

Edit: Added counts

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Verified on local. Thanks!

@archseer archseer merged commit 47d2e3a into helix-editor:master Jun 17, 2021
@sudormrfbin sudormrfbin deleted the fix-open-above-gg branch June 17, 2021 10:03
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.

Using O on the first line does not move the cursor
3 participants