Cmdline history: deque, restore input command, ability to use up/down arrow keys #600

raphCode · 2023-04-25T22:31:57Z

This PR aims to change three things related to the internal command line history:

using collections.deque for the history storage: This handles the maximum size internally, automatically deleting old entries on append()
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.
Restore command line input when browsing history: See commit messages and in-code comments for detailed explanation
This fixes one of my pain points in pudb: When accidentally hitting Ctrl-N in the command line, it deletes my potentially long command input with no possibility to get it back.
Using up/down arrow keys for browsing history: This is not implemented yet as I want to ask how I should implement this. Is adding a boolean config option fine that enables this?
This is my other pain point in the pudb internal shell: After not being aware for a year that browsing the history is even possible, I still find Ctrl-N/P highly unfamiliar despite using pudb for multiple years now. I still press the arrow keys now and then.

inducer · 2023-04-26T17:16:49Z

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.)

raphCode · 2023-04-26T18:55:26Z

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.

inducer · 2023-05-03T04:33:42Z

I think the instant-apply logic for that setting will be cumbersome/hard to maintain. Let's just switch it over.

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: inducer#600 (comment)

raphCode · 2023-05-12T09:05:15Z

I added the arrow keys to the existing Ctrl-n/p keys for history browsing.
I also rephrased the help a bit to avoid confusion and reflect the new behavior.
I was using my modifications the last weeks and I really like them.
I added one minor fix / improvement: "Always recall the just executed command on arrow up".

One thing still remains: I am not sure what the behavior should be when:

browsing to a historic command
editing the prompt
browsing up/down the history
Currently, the next history element is recalled, overwriting all modifications to the prompt, loosing the text.
This is probably a rare edge case.

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?

inducer · 2023-05-17T16:55:19Z

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.

raphCode · 2023-05-17T17:25:00Z

So the idea is to scroll / browse the results scrollback, but leaving the focus in the prompt? Interesting, I will try that.

raphCode · 2023-05-19T10:04:41Z

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 ListBoxes 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.

inducer · 2023-05-21T22:40:04Z

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.

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: inducer#600 (comment)

raphCode · 2023-05-31T14:45:47Z

✔️ rebased on main
✔️ figured out how to make the shift page up/down scrolling work.
I think I needed some time to understand the urwid way of doing thing :D

Let me know what you think!

raphCode · 2023-08-30T07:40:04Z

Hi! Just wondering if there's still anything to be done on my end do get this merged?
(except maybe merge / rebase on main)

inducer

This LGTM. Could you rebase and add docs for Shift+Pg{Up,Dn}?

inducer · 2023-08-30T19:16:16Z

pudb/debugger.py

+        for key in ("page up", "page down"):
+            self.cmdline_sigwrap.listen("shift " + key, cmdline_results_scroll)


Could you document these keystrokes?

A deque handles the size limit internally, eliminating some logic in the debugger itself.

Text input to the internal command line is now restored and not lost after browsing the history. It is designed to behave just like a dedicated history item at the end of the history. This matches the behavior of readline in most shells. For example, writing a command without executing it, then moving up to the most recent history item and down again restores the typed command. The pudb special behavior of clearing the command when moving down from the end of the history is also preserved. In that case the text can be restored too by moving up in the history again before typing anything new. This is akin to using Ctrl-C in common shells to clear the current command, but with the ability to restore the input again. Using the clear button which empties the displayed shell history now also clears the command input, since it can be easily restored. This fixes one of my pain points with the internal pudb command line: When writing a lengthy command, accidentally hitting Ctrl-N or Ctrl-P overwrites the input text field with no possibility to get the content back. This is particularly bad after using vim's word completion with Ctrl-N for an extensive amount of time, and then accidentally hitting the same shortcut in pudb instead of tab completion.

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: inducer#600 (comment)

There is the - command history, which stores previously executed commands - result scrollback, which refers to list of shell prompts and their results The latter one was previously called 'command line history'. To avoid confusion, this commit introduces a major new term.

After executing a command, immediatly hitting the arrow up key should recall the just executed command again, like in all shells. This commit ensures this behavior by clearing out the saved edit text on successful command execution. Previously, it recalled the last saved edit text if set, which is an annoying workflow break compared to other shells.

raphCode · 2023-09-15T09:22:11Z

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.

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: #600 (comment)

inducer · 2023-09-18T01:11:06Z

Thanks!

inducer marked this pull request as draft April 26, 2023 17:16

raphCode force-pushed the history-improvements branch from 6c4eeeb to 536db4a Compare April 26, 2023 18:56

raphCode force-pushed the history-improvements branch from 536db4a to 8833e50 Compare May 12, 2023 08:43

raphCode force-pushed the history-improvements branch from 8833e50 to 43605e5 Compare May 12, 2023 08:48

raphCode force-pushed the history-improvements branch from 43605e5 to 6c0c11f Compare May 31, 2023 14:41

raphCode marked this pull request as ready for review May 31, 2023 14:42

raphCode force-pushed the history-improvements branch from 6c0c11f to 16d2c97 Compare May 31, 2023 14:47

inducer approved these changes Aug 30, 2023

View reviewed changes

raphCode added 7 commits September 15, 2023 11:12

Use deque with maxsize for command line history

83dda3b

A deque handles the size limit internally, eliminating some logic in the debugger itself.

Use arrow keys for command history browsing

e81ab07

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: inducer#600 (comment)

Scroll cmdline results on shift page up/down

a88414e

Document shift page up/down

6a1e11d

raphCode force-pushed the history-improvements branch from 16d2c97 to 6a1e11d Compare September 15, 2023 09:12

inducer merged commit 55a8d75 into inducer:main Sep 18, 2023

inducer pushed a commit that referenced this pull request Sep 18, 2023

Use arrow keys for command history browsing

ea7981b

This changes the arow keys from accessing the command line results to browsing the command history, as requested by the main developer: #600 (comment)

inducer mentioned this pull request Mar 8, 2024

Map up/down keys to command history prev/next #392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cmdline history: deque, restore input command, ability to use up/down arrow keys #600

Cmdline history: deque, restore input command, ability to use up/down arrow keys #600

raphCode commented Apr 25, 2023

inducer commented Apr 26, 2023

raphCode commented Apr 26, 2023 •

edited

Loading

inducer commented May 3, 2023

raphCode commented May 12, 2023

inducer commented May 17, 2023

raphCode commented May 17, 2023

raphCode commented May 19, 2023

inducer commented May 21, 2023

raphCode commented May 31, 2023 •

edited

Loading

raphCode commented Aug 30, 2023 •

edited

Loading

inducer left a comment

inducer Aug 30, 2023

raphCode commented Sep 15, 2023 •

edited

Loading

inducer commented Sep 18, 2023

		for key in ("page up", "page down"):
		self.cmdline_sigwrap.listen("shift " + key, cmdline_results_scroll)

Cmdline history: deque, restore input command, ability to use up/down arrow keys #600

Cmdline history: deque, restore input command, ability to use up/down arrow keys #600

Conversation

raphCode commented Apr 25, 2023

inducer commented Apr 26, 2023

raphCode commented Apr 26, 2023 • edited Loading

inducer commented May 3, 2023

raphCode commented May 12, 2023

inducer commented May 17, 2023

raphCode commented May 17, 2023

raphCode commented May 19, 2023

inducer commented May 21, 2023

raphCode commented May 31, 2023 • edited Loading

raphCode commented Aug 30, 2023 • edited Loading

inducer left a comment

Choose a reason for hiding this comment

inducer Aug 30, 2023

Choose a reason for hiding this comment

raphCode commented Sep 15, 2023 • edited Loading

inducer commented Sep 18, 2023

raphCode commented Apr 26, 2023 •

edited

Loading

raphCode commented May 31, 2023 •

edited

Loading

raphCode commented Aug 30, 2023 •

edited

Loading

raphCode commented Sep 15, 2023 •

edited

Loading