Add support for DECBKM (Backarrow Key Mode) #13894
Conversation
ghost
added
Issue-Task
| // GH#12799 - If the app requested that we disable focus events, DON'T pass | ||
| // that through. ConPTY would _always_ like to know about focus events. | ||
|
|
||
| return !_api.IsConsolePty() || | ||
| !_api.IsVtInputEnabled() || | ||
| (!enable && mode == TerminalInput::Mode::FocusEvent); | ||
|
|
||
| // Another way of writing the above statement is: | ||
| // | ||
| // const bool inConpty = _api.IsConsolePty(); | ||
| // const bool shouldPassthrough = inConpty && _api.IsVtInputEnabled(); | ||
| // const bool disabledFocusEvents = inConpty && (!enable && mode == TerminalInput::Mode::FocusEvent); | ||
| // return !shouldPassthrough || disabledFocusEvents; | ||
| // | ||
| // It's like a "filter" left to right. Due to the early return via | ||
| // !IsConsolePty, once you're at the !enable part, IsConsolePty can only be | ||
| // true anymore. | ||
| return !_api.IsConsolePty() || !_api.IsVtInputEnabled(); |
There was a problem hiding this comment.
Note to reviewers, if you didn't read the PR description, the additional check here that was specific to the FocusEvent mode has now been moved down into the EnableFocusEventMode method. I think this reduces some of the confusion that was introduced here, and also avoids unnecessary overhead in the other input mode operations.
This is isn't strictly related to DECBKM mode, though (other than as a minor optimisation), so if you'd prefer not to include this change in the PR, it wouldn't be a problem to revert it.
There was a problem hiding this comment.
This makes way more sense. Thanks!
| // GH#12799 - If the app requested that we disable focus events, DON'T pass | ||
| // that through. ConPTY would _always_ like to know about focus events. | ||
|
|
||
| return !_api.IsConsolePty() || | ||
| !_api.IsVtInputEnabled() || | ||
| (!enable && mode == TerminalInput::Mode::FocusEvent); | ||
|
|
||
| // Another way of writing the above statement is: | ||
| // | ||
| // const bool inConpty = _api.IsConsolePty(); | ||
| // const bool shouldPassthrough = inConpty && _api.IsVtInputEnabled(); | ||
| // const bool disabledFocusEvents = inConpty && (!enable && mode == TerminalInput::Mode::FocusEvent); | ||
| // return !shouldPassthrough || disabledFocusEvents; | ||
| // | ||
| // It's like a "filter" left to right. Due to the early return via | ||
| // !IsConsolePty, once you're at the !enable part, IsConsolePty can only be | ||
| // true anymore. | ||
| return !_api.IsConsolePty() || !_api.IsVtInputEnabled(); |
There was a problem hiding this comment.
This makes way more sense. Thanks!
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
🎉 Handy links: |
This PR adds support for the
DECBKMsequence, which provides a way forapplications to specify whether the
Backspacekey should generate aBScharacter or aDELcharacter.On the original VT100 terminals, the
Backarrowkey generated aBS,and there was a separate
Deletekey that generatedDEL. However, onthe VT220 and later, there was only the
Backarrowkey. By default itgenerated
DEL, but from the VT320 onwards there was an option to makeit generate
BS, and theDECBKMsequence provided a way to controlthat behavior programmatically.
On modern terminals, the
Backspacekey serves as the equivalent of theVT
Backarrow, and typically generatesDEL, whileCtrl+Backspacegenerates
BS. WhenDECBKMis enabled (for those that support it),that behavior is reversed, i.e.
BackspacegeneratesBSandCtrl+BackspacegeneratesDEL.This PR also gets the other
Backspacemodifiers more closely alignedwith the expected behavior for modern terminals. The
Shiftmodifiertypically has no effect, and the
Altmodifier typically prefixes thegenerated sequence with an
ESCcharacter.While not strictly related to
DECBKM, I noticed while testing that the_SetInputModemethod was doing unnecessary work that was specific tothe
FocusEventmode. I've now moved that additional processing intothe
EnableFocusEventModemethod, which I think makes things somewhatsimpler.
Validation Steps Performed
I've tested the basic
DECBKMfunctionality in Vttest, and I'vemanually tested all the modifier key combinations to make sure they
match what most modern terminals generate.
I've also added a unit test that confirms that the expected sequences
are generated correctly when the
DECBKMmode is toggled.Closes #13884