Avoid passing commands through with the shift modifier

[ellie-idb]

Summary of the Pull Request

This change would enable keybinds like Ctrl+Shift+C or Ctrl+Shift+V to work, as they would be captured inadvertently, and sent as their respective escape sequences.

References

#2000

PR Checklist

• CLA signed. If not, go over here and sign the CLA
• Tests added/passed

Validation Steps Performed

Built Terminal on separate instance of Azure Pipelines, then validated that functionality (Ctrl+Shift+V/Ctrl+Shift+C) work as intended (which is, that they properly copy and paste)

[zadjii-msft]

I'm almost certain this isn't the right fix. This would change the behavior of input in conhost.exe, not the Windows Terminal. Could you provide better explanation of why you're making this change at this location in the code? Will this regress any existing behavior?

[ellie-idb]

@zadjii-msft Typically, any control sequence (that is, ^C, ^D, etc) does not (and should not, you can test this with showkey -a on any terminal emulator in an Linux environment) require the use of the Shift key. No behavior should be regressed from this, and it's just intended to enable an extra layer of keybinding (i.e Ctrl+Shift+C, Ctrl+Shift+V)

[DHowett-MSFT]

However, this low-level input processor is shared between conhost and Windows Terminal. Should Shift be stripped off in the much higher-level TerminalControl layer?

[zadjii-msft]

Aye, but there are already keybindings that work just fine in conhost on Ctrl+Shift+V and Ctrl+Shift+C.

Does this affect INPUT_RECORDs that are generated for console applications reading input with ReadConsoleInput?

This might be more correct for just the terminal portion of conhost, but I'm worried about the console bits of conhost regressing.

If this does change the behavior, we should probably add a test to cover it, since it's clearly not covered currently :P

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[ellie-idb]

I believe this is already fixed higher up in the input chain (rather then directly at conhost), seemingly inadvertently with #2235
See:

terminal/src/cascadia/TerminalControl/TermControl.cpp

Lines 643 to 652 in 4529e46

	auto bindings = _settings.KeyBindings();
	if (bindings)
	{
	handled = bindings.TryKeyChord({
	modifiers.IsCtrlPressed(),
	modifiers.IsAltPressed(),
	modifiers.IsShiftPressed(),
	vkey,
	});
	}