Open eugenesvk opened 3 years ago
Hi @eugenesvk,
Actually this makes sense. Feel free to add c-backspace
and add support for Windows and create a PR. I'm not sure how standardized this is in Linux terminals, so there we could decide to not map any sequence to this key.
By the way, Keys.Backspace
is an alias for Keys.ControlH
. Both resolve to c-h
.
I'm not sure whether this is something we should change. Seperating them would actually be a breaking change.
Feel free to add
c-backspace
and add support for Windows and create a PR.
Ok, will do the Win part separately Any preference re.
Keys.ControlBackspace
vsCorrectly handle Control-Arrow/Home/End and Control-Insert/Delete/Backspace
function (or making a separate one like mentioned in the original comment)To me, the first option seems more logical since the b"\x7f"
is actually emitted by Ctrl+BS, so it should also map to a Keys.ControlBackspace
rather than to a Keys.Backspace
and then requiring a separate handling of Ctrl
Also, I'd like to add Shift/ControlShiftBackspace combos to a better coverage of this backward key :) ShiftBackspace here https://github.com/prompt-toolkit/python-prompt-toolkit/blob/d6ac8be0791184d87262d5a759b11664dcdbf110/prompt_toolkit/input/win32.py#L372
ControlShiftBackspace here https://github.com/prompt-toolkit/python-prompt-toolkit/blob/d6ac8be0791184d87262d5a759b11664dcdbf110/prompt_toolkit/input/win32.py#L350
As far as I understand, for the Win part that's it, I don't need to add any new @handle("c-backspace")
lines to the basic.py
, just keys.py
and win32.py
will suffice?
By the way,
Keys.Backspace
is an alias forKeys.ControlH
. Both resolve toc-h
.I'm not sure whether this is something we should change. Seperating them would actually be a breaking change.
Is this something that can be optional? Like, is it possible to unalias/add the required changes to the ansi_escape_sequences.py
and keys.py
dicts/classes on loading your toolkit? Or is it too basic and there can only be one way?
Although I'd still argue it's much better to go the more key-configurable way and be able to treat the Backspace and ControlBackspace separately, so it's fine to break compatibility with those backward terminals that don't support it :) That's why I thought that the change of direction in that blog was relevant to support this change as well (if I understood it correctly)
By the way, re. those terminals, do we know if this is still true: "Most terminals send ControlH when backspace is pressed."? Have modern/top Linux terminals progressed enough to be able to differentiate (only using Terminator on Linux, and it does send 0x7f
on Backspace; on WSL Windows Terminal/ConEmu also send it, but no idea about the others)
I'm not sure how standardized this is in Linux terminals, so there we could decide to not map any sequence to this key.
Yeah, that's fine, though wouldn't it be better to have a default mapping that can be easily changed by those who have a different standard, rather than no default mapping requiring everyone to change it (unless no one uses it and expect nothing to happen on ^BS :), then the default should probably also be no binding)?
I've now submitted two patches that cover both the Windows and the non-Windows parts of this issue (left the non-Windows ^H
bound to backward-delete-char
, not to backward-kill-word
as I'm not sure if it'd break anything there) and tested them in xonsh shell on Windows/WSL/Ubuntu/macOS with WinTerminal/ConEmu/Terminator/iTerm2, seems to be working as expected — I can finally get my ControlBackspace functionality everywhere!
I see that there are two items in the test_cli.py
failing, so if we don't want to add handle("c-h", filter=insert_mode)(get_by_name("backward-kill-word"))
to emacs.py
to make them pass I guess those need to be removed as Esc+^H
wouldn't be expected to backward-kill-word
like Esc+BS
does?
First let me thank you for this project. So far it looks like a nice library to work with, and the code I've read looks really clean :smiley:
But I want to add my strong wish a mechanism to allow this to work on Linux as well. (To me personally it's really an showstopper for using the library)
Ctrl-backspace is universally supported (that is - everywhere besides some terminal applications) and is thus very ingrained in my fingers. And I imagine many others. I do understand (on a shallow level) that such keys are annoying and hard to deal with in terminals, but multiple applications make it work. Even if that means some cooperation with the specific terminal emulator and some knobs that need turning. (one such possible knob would be to allow the application to pass in custom vt100 parser, or simpler, a custom ANSI_SEQUENCES mapping)
I also want to challenge this claim:
... with the trade-off that it's no longer possible to handle backspace and control-h individually for the few terminals that support it
Is it really still few? All the terminal emulators I've used have supported this as far as I can remember.
I can try implementing a suggestion for some sort of "knob" if I know there's a chance it can be accepted.
In the meantime I'll simply hack around it like this:
from prompt_toolkit.input.ansi_escape_sequences import ANSI_SEQUENCES
from prompt_toolkit.keys import Keys
ANSI_SEQUENCES["\x08"] = Keys.F24
kb = KeyBindings()
@kb.add(Keys.F24)
def _kill_word(event: KeyPressEvent):
# https://github.com/prompt-toolkit/python-prompt-toolkit/issues/1380
b = named_commands.get_by_name("unix-word-rubout")
return b.handler(event)
One of my main usability issues with xonsh that's relying on your prompt toolkit is that I can't remap a very common key combo — ControlBackspace — to delete a previous word
There is already a similar issue that was closed with a reference to your comment; and as far as I understood, with the relevant code with some additional justification here: https://github.com/prompt-toolkit/python-prompt-toolkit/blob/d6ac8be0791184d87262d5a759b11664dcdbf110/prompt_toolkit/input/ansi_escape_sequences.py#L58
Question 1
Is there any chance to revert this bundling of two keys into one key code? The blog mentioned above as part of the rationale for this seems to have changed its approach and (though I'm not certain) to now favor separating backspace and control-h so that "^H ... can be used by applications" (which I assume would allow mapping ControlBackspace separately from Backspace)
Or at least make this optional/configurable so that a user can decide whether the trade-off is worth it?
Question 2
Regardless of the general solution, is it possible to achieve the same in Windows, where
b"\x7f"
is already mapped toKeys.Backspace
, notKeys.ControlH
? https://github.com/prompt-toolkit/python-prompt-toolkit/blob/d6ac8be0791184d87262d5a759b11664dcdbf110/prompt_toolkit/input/win32.py#L148I'm not really sure how exactly all these mappings work and what the potential downsides are, but I've made just a couple of very minor changes in this fork
ControlBackspace = "c-backspace"
tokeys.py
b"\x7f": Keys.Backspace,
withKeys.ControlBackspace
inwin32.py
...and I'm able to map ControlBackspace to delete previous word just fine in xonsh (using this function with the key changed to
ControlBackspace
)Alternatively, leaving the
b"\x7f"
as is and adding a function similar toTurn 'Space' into 'ControlSpace' when control was pressed.
also seems to work:Are there any major issues with this simple addition of ControlBackspace that would prevent adding this change to your toolkit?