Implement basic preview scrolling for FilePicker #14839
Conversation
|
I wouldn't normally bikeshed over key choice, but would CTRL+d and CTRL+u make more sense, as popups already use these keys for scrolling? |
helix-term/src/ui/picker.rs
Outdated
| self.toggle_preview(); | ||
| } | ||
| ctrl!('j') => { | ||
| self.scroll.position += 1; |
There was a problem hiding this comment.
I think scrolling one line at a time is probably too little, no? I'd personally make it scroll half the current page size, similar to z + space,backspace
There was a problem hiding this comment.
I don't know. Most of the time I just need to look around a bit, not scroll an entire document up and down. If it will not be rejected right away, we'll make the scrolling step configurable plus add hotkeys for half-page scrolling, no problem.
ClassAteam
force-pushed
the
picker-explorer
branch
from
5836ee0 to
aa8d6e1
Compare
|
I've updated the commit. Now preview works for every picker. The solution I was originally directed to looks a bit complex and obscure in my opinion. So let this one live for a while until a qualified member reviews and closes it if needed. |
helix-term/src/ui/picker.rs
Outdated
| self.toggle_preview(); | ||
| } | ||
| ctrl!('j') => { | ||
| self.scroll.position += 1; |
There was a problem hiding this comment.
This needs to be capped at doc.len_lines(). Scrolling past the end of the document causes a panic.
There was a problem hiding this comment.
Thanks for the review. I've fixed the capping issue. Should i add an integration test for this tiny change ?
There was a problem hiding this comment.
No I think an integration test for this would be overkill
ClassAteam
force-pushed
the
picker-explorer
branch
from
aa8d6e1 to
f265126
Compare
ClassAteam
force-pushed
the
picker-explorer
branch
from
f265126 to
60aa562
Compare
the-mikedavis
left a comment
the-mikedavis
left a comment
There was a problem hiding this comment.
Like #11441 this doesn't work well if soft-wrap is enabled. With :set soft-wrap.enable true if you preview a doc with a very long last line, you can only see the beginning of the line. Rather than scrolling by document lines, the scrolling should work on virtual lines (after wrapping)
4 participants
Behavior
After pressing SPC f, the file picker for all files in a project is created. The picker also provides a preview widget on the right by default. This patch allows scrolling up and down this preview using Ctrl + j / Ctrl + k keys.
Reason
Seems like a pretty obvious and necessary feature for file picker preview. I guess I'm not aware of the deep reasons why it wasn't implemented already. So I create this PR as a proposition for further, more thorough work and look forward to it being rejected with a slap and a link showing that I missed something.