Closed raphCode closed 1 year ago
This is not implemented yet
Converting to draft to avoid mistakes.
Is adding a boolean config option fine that enables this?
I agree that arrow keys are more discoverable. Let's just switch it over for everyone. Maybe add shift-page up/down (or so) for history scrollback. (It should still also be available as Cursor right (go to 'clear' button) and then up/down.)
I agree that arrow keys are more discoverable. Let's just switch it over for everyone.
In case you want a setting option, I started to hack something together: It's not finished yet, so I am fine with discarding the setting and just changing it for everyone. But I fear it might break someone's workflow when changing the keybinding after so long.
Maybe add shift-page up/down (or so) for history scrollback.
Sure. Btw, I am in favor of calling it the "result scrollback" or something to avoid confusion with the command history.
It should still also be available as Cursor right (go to 'clear' button) and then up/down.
It is. I also intended to write it next to the proposed setting, like in the screenshot.
I think the instant-apply logic for that setting will be cumbersome/hard to maintain. Let's just switch it over.
One thing still remains: I am not sure what the behavior should be when:
Maybe add shift-page up/down (or so) for history scrollback.
How would I do that? Add a keybinding that calls set_focus()
on the result scrollback list? How can I then move the focused element there?
I am not sure what the behavior should be when:
I think that's fine.
How would I do that?
How about just send a "Page-Up" keypress to the widget? (without changing focus) Not sure if that'll work, but it may.
So the idea is to scroll / browse the results scrollback, but leaving the focus in the prompt? Interesting, I will try that.
How about just send a "Page-Up" keypress to the widget?
I am having trouble sending a keypress to an unfocused widget.
Calling keypress()
seems to need a tuple defining the widget's size. From the urwid manual I read that sizes are calculated on the go internally and not really exposed to the user code.
Alternatively, the docs recommend to scroll ListBox
es with set_focus()
that can be called with a position only (no size), but that jumps result-wise. Meaning if a result spans more than the visible scrollback height, scrolling is not satisfactory and skips parts of the result.
A related question: https://stackoverflow.com/questions/49416332/focus-widget-in-list-but-keep-cursor-in-another/49520580, but that seems to jump result-wise again.
I think the instant-apply logic for that setting will be cumbersome/hard to maintain.
By researching the keypress stuff, I stumbled across this: http://urwid.org/reference/command_map.html#urwid.CommandMap I reads like that could help, but I didn't try.
Thanks for exploring. Given the apparent difficulty, I think it might be best to leave making the history scrollback more efficient out of scope for now.
Let me know what you think!
Hi! Just wondering if there's still anything to be done on my end do get this merged? (except maybe merge / rebase on main)
Sorry for the delay. I added docs and rebased it, it should merge now.
While testing the new results browse function, I experienced some UI glitches sometimes. The seems to be a strange combination of urwid, my terminal multiplexer zellij (UI must be unlocked) and the keystrokes shift+up/down. This is not an pudb bug and it happened before this PR as well. Nothing too serious to worry about, but maybe something to keep in mind when someone complains.
Thanks!
This PR aims to change three things related to the internal command line history:
collections.deque
for the history storage: This handles the maximum size internally, automatically deleting old entries onappend()
I just found a deque a nice fit for the use case while working on the other features, but there is no absolute need for this change.