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

Scroll cursor and page together (neovim-like scrolling) #8015

Merged
merged 13 commits into from
Feb 18, 2024

Conversation

AlexanderDickie
Copy link
Contributor

@AlexanderDickie AlexanderDickie commented Aug 20, 2023

Solves #7639 - adds a new scroll function so that the cursor and page jump by the same distance vertically. Assigns this to C-u and C-d, similar to neovim.

The consistent distance between cursor and view top would break if the cursor were to land on a whole line of virtual text- the cursor would be placed on the closest non-virtual text above where it landed. Meaning the view and cursor would not have jumped the same vertical distance.
This should be pretty rare though- unless you are using #6417 and have a ton of error messages. I could refactor this nvim-scroll function though- to first calculate how many lines the cursor jumped (eg the cursor tried to jump by half a page but could only jump by half a page -1 line due to virtual text) and then move the view by exactly this number. This would change the behaviour at the bottom of the page a bit also.

This nvim-scroll function respects the cursor's column throughout scrolls which I think is nice, I will make a pr for the scroll function to have the same.

Closes #7639

@pascalkuthe pascalkuthe added E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-command Area: Commands labels Aug 21, 2023
pascalkuthe
pascalkuthe previously approved these changes Aug 21, 2023
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

very nice this is now a very small/clean change that adds something that I find very useful. Thanks for working on this!

@pascalkuthe
Copy link
Member

looks like formatting still needs to be fixed tough

@pascalkuthe
Copy link
Member

(and some clippy lints)

pascalkuthe
pascalkuthe previously approved these changes Aug 21, 2023
@wildwestrom
Copy link
Contributor

Does this require the documentation to be updated?

@AlexanderDickie
Copy link
Contributor Author

Can we get the labels changed on this so it gets reviewed? 😄

@pascalkuthe pascalkuthe added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 1, 2023
@pascalkuthe pascalkuthe added this to the next milestone Sep 1, 2023
@pascalkuthe
Copy link
Member

The labels don't really affect what we review too much. It's mostly for other people

@pascalkuthe pascalkuthe merged commit 9ab3f9d into helix-editor:master Feb 18, 2024
6 checks passed
uek-1 pushed a commit to uek-1/helix that referenced this pull request Feb 24, 2024
…#8015)

* neovim like scroll function

* clear line annotations outside of move_vertically/_visual

* add nvim scroll function to commands

* assign nvim-scroll to C-d and C-u (half page scrolls)

* dont remove backspace and space mapping

* move non-softwrap logic to seperate function, call this in nvim-scroll fn

* Revert "move non-softwrap logic to seperate function, call this in nvim-scroll fn"

This reverts commit e490572.

* Revert "clear line annotations outside of move_vertically/_visual"

This reverts commit 1df3fef.

* add TODO for when inline diagnostics gets merged

* move nvim-scroll logic into scroll(), dont respect scrolloff

* run cargo fmt

* run cargo clippy

* update documenation for Ctrl-d and Ctrl-u remap
2639-unofficial added a commit to 2639-unofficial/krustallos that referenced this pull request Mar 1, 2024
C-u/d config is no longer needed after helix-editor/helix#8015
shortc pushed a commit to shortc/helix that referenced this pull request Mar 31, 2024
…#8015)

* neovim like scroll function

* clear line annotations outside of move_vertically/_visual

* add nvim scroll function to commands

* assign nvim-scroll to C-d and C-u (half page scrolls)

* dont remove backspace and space mapping

* move non-softwrap logic to seperate function, call this in nvim-scroll fn

* Revert "move non-softwrap logic to seperate function, call this in nvim-scroll fn"

This reverts commit e490572.

* Revert "clear line annotations outside of move_vertically/_visual"

This reverts commit 1df3fef.

* add TODO for when inline diagnostics gets merged

* move nvim-scroll logic into scroll(), dont respect scrolloff

* run cargo fmt

* run cargo clippy

* update documenation for Ctrl-d and Ctrl-u remap
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
…#8015)

* neovim like scroll function

* clear line annotations outside of move_vertically/_visual

* add nvim scroll function to commands

* assign nvim-scroll to C-d and C-u (half page scrolls)

* dont remove backspace and space mapping

* move non-softwrap logic to seperate function, call this in nvim-scroll fn

* Revert "move non-softwrap logic to seperate function, call this in nvim-scroll fn"

This reverts commit e490572.

* Revert "clear line annotations outside of move_vertically/_visual"

This reverts commit 1df3fef.

* add TODO for when inline diagnostics gets merged

* move nvim-scroll logic into scroll(), dont respect scrolloff

* run cargo fmt

* run cargo clippy

* update documenation for Ctrl-d and Ctrl-u remap
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
…#8015)

* neovim like scroll function

* clear line annotations outside of move_vertically/_visual

* add nvim scroll function to commands

* assign nvim-scroll to C-d and C-u (half page scrolls)

* dont remove backspace and space mapping

* move non-softwrap logic to seperate function, call this in nvim-scroll fn

* Revert "move non-softwrap logic to seperate function, call this in nvim-scroll fn"

This reverts commit e490572.

* Revert "clear line annotations outside of move_vertically/_visual"

This reverts commit 1df3fef.

* add TODO for when inline diagnostics gets merged

* move nvim-scroll logic into scroll(), dont respect scrolloff

* run cargo fmt

* run cargo clippy

* update documenation for Ctrl-d and Ctrl-u remap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Neovim-like exact scrolling
5 participants