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

Redone #4189, working preview scrolling #11441

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

GNUSheep
Copy link
Contributor

@GNUSheep GNUSheep commented Aug 7, 2024

Finally redone preview scrolling on current master. Works pretty fine.

I used code from: @Manosmer, and make it work with current version of helix.

Closes #4102

@GNUSheep GNUSheep changed the title Redone helix-editor#4189, working preview scrolling Redone #4189, working preview scrolling Aug 7, 2024
@thomasaarholt
Copy link
Contributor

Current keybindings:

  • alt-j / shift-down - Move preview down scroll-lines at a time
  • alt-k / shift-up - Move preview up scroll-lines at a time
  • alt-d - Move preview down half height of the preview area
  • alt-u - Move preview up half height of the preview area
  • alt-f / alt-PageDown - Move preview down one full preview area height
  • alt-b / alt-PageUp - Move preview up one full preview area height

Originally posted by @EpocSquadron in #4189 (comment)

@thomasaarholt
Copy link
Contributor

I do observe that when scrolling, the syntax highlighting does not update. Only the text first shown in the preview pane is syntax highlighted.

Screenshot 2024-08-18 at 19 15 50

@GNUSheep GNUSheep force-pushed the scroll_preview_fixed branch from 05e8d71 to ecd415f Compare August 19, 2024 12:39
@GNUSheep GNUSheep force-pushed the scroll_preview_fixed branch from ecd415f to a69ea1d Compare August 19, 2024 13:00
@GNUSheep
Copy link
Contributor Author

@thomasaarholt

Hi! Thank you for your help. I fixed my mistakes, and now all should work well.

My only concern is about keybinding alt-PageUp, because as mentioned before:

I'm not 100% certain alt- is valid with PageUp/PageDown in terminals, but worth a try.

I tried the above on iTerm2 without success. Therefore, I did not include those keybindings. It might be a limitation just on my platform though.

In my case alt-PageDown works pretty fine, but alt-PageUp doesn't.
I tested on alattracity, and gnome terminal and got same result.

But as mentioned above it might be just a limitation on my platform.

@kirawi kirawi added the A-helix-term Area: Helix term improvements label Aug 28, 2024
@GNUSheep
Copy link
Contributor Author

@archseer
@the-mikedavis

@EmrysMyrddin
Copy link

I've tried it, works very well, very welcomed addition ! I hope it will be merged soon :-)

@EmrysMyrddin
Copy link

I'm using Helix based on your PR, and I came across another issue:
image

Here the diagnostic is miss-rendered. It should not underline the whole comment.

Here how it looks in the main buffer:
image

Note: the diagnostic is rendered in the preview only if the file have been opened at least once since Helix started.

@David-Else
Copy link
Contributor

@thomasaarholt

My only concern is about keybinding alt-PageUp, because as mentioned before:

I'm not 100% certain alt- is valid with PageUp/PageDown in terminals, but worth a try.

I tried the above on iTerm2 without success. Therefore, I did not include those keybindings. It might be a limitation just on my platform though.

In my case alt-PageDown works pretty fine, but alt-PageUp doesn't. I tested on alattracity, and gnome terminal and got same result.

But as mentioned above it might be just a limitation on my platform.

I am not a fan of using the Alt key, particularly with the Alt + PageUp/PageDown combinations. I have noticed that currently, PageUp/PageDown seems to jump up and down the file or item list by a seemingly random amount in different pickers. I really can't find a good use for the current functionality in the picker. It would be better to use PageUp/PageDown for preview scrolling instead, similar to how LazyGit does. Now that we have Pickers v2, it makes more sense to refine the search rather than scrolling through the list of items in such large increments.

@GNUSheep
Copy link
Contributor Author

GNUSheep commented Sep 20, 2024

@EmrysMyrddin

Hi! Thank u for using helix based on my PR.
I fixed that bug,
(I just forgot to change offset.anchor in doc_diagnostics_highlights)

image
image

If u find more bugs I will be happy to fix them :)

@GNUSheep
Copy link
Contributor Author

@David-Else
Hello! Looking at the code PageUp and PageDown, moves up or down by a exactly one page. But I agree with you that it is not so useful, I think that many people just search by using search bar rather than scrolling up.

Using just PageUp or PageDown seems like nice idea.

@GNUSheep
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Picker preview scrolling
5 participants