Skip to content

Commit

Permalink
add remaining notes from PR
Browse files Browse the repository at this point in the history
  • Loading branch information
@zadjii-msft
zadjii-msft committed Jun 3, 2020
1 parent 9c17236 commit 70fbe9b
Showing 1 changed file with 52 additions and 7 deletions.
59 changes: 52 additions & 7 deletions doc/specs/#4999 - Improved keyboard handling in Conpty.md
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
author: Mike Griese @zadjii-msft
created on: 2020-05-07
last updated: 2020-05-20
last updated: 2020-06-03
issue id: 4999
---

Expand Down Expand Up @@ -141,9 +141,12 @@ typedef struct _KEY_EVENT_RECORD {
```
To encode all of this information, I propose the following sequence. This is a
CSI sequence with a final terminator character of `_`. This character appears
unused as a terminator by sequences _output_ by a client application, and
doesn't seem to be used as an _input_ sequence terminator either.
CSI sequence with a final terminator character of `_`. This character appears to
only be used as a terminator for the [SCO input
sequence](https://vt100.net/docs/vt510-rm/chapter6.html) for
<kbd>Ctrl+Shift+F10</kbd>. This conflict isn't a real concern for us
compatibility wise. For more details, see [SCO
Compatibility](#sco-compatibility) below.
```
^[ [ Vk ; Sc ; Uc ; Kd ; Cs ; Rc _
Expand Down Expand Up @@ -350,6 +353,46 @@ this ConPTY change. The Terminal will only switch into sending
`win32-input-mode` sequences when _conpty asks for them_. Otherwise, the
Terminal will still behave like a normal terminal emulator.
#### Terminals that don't support `?9001h`
Traditionally, whenever a terminal emulator doesn't understand a particular VT
sequence, they simply ignore the unknown sequence. This assumption is being
relied upon heavily, as ConPTY will _always_ emit a `^[[?9001h` on
initialization, to request `win32-input-mode`.
#### SCO Compatibility
As mentioned above, the `_` character is used as a terminator for the [SCO input
sequence](https://vt100.net/docs/vt510-rm/chapter6.html) for
<kbd>Ctrl+Shift+F10</kbd>. This conflict would be a problem if a hypothetical
terminal was connected to conpty that sent input to conpty in SCO format.
However, if that terminal was only sending input to conpty in SCO mode, it would
have much worse problems than just <kbd>Ctrl+Shift+F10</kbd> not working. If we
did want to support SCO mode in the future, I'd even go so far as to say we
could maybe treat a `win32-input-mode` sequence with no params as
<kbd>Ctrl+Shift+F10</kbd>, considering that `KEY_EVENT_RECORD{0}` isn't really
valid anyways.
#### Remoting `INPUT_RECORD`s
A potential area of concern is the fact that VT sequences are often used to
remote input from one machine to another. For example, a terminal might be
running on machine A, and the conpty at the end of the pipe (which is running
the client application) might be running on another machine B.
If these two machines have different keyboard layouts, then it's possible that
the `INPUT_RECORD`s synthesized by the terminal on machine A won't really be
valid on machine B. It's possible that machine B has a different mapping of scan
codes \<-> characters. A client that's running on machine B that uses win32 APIs
to try and infer the vkey, scancode, or character from the other information in
the `INPUT_RECORD` might end up synthesizing the wrong values.
At the time of writing, we're not really sure what a good solution to this
problem would be. Client applications that use `win32-input-mode` should be
aware of this, and be written with the understanding that these values are
coming from the terminal's machine, which might not necessarily be the local
machine.
### Performance, Power, and Efficiency
_(no change expected)_
Expand All @@ -374,6 +417,8 @@ _(no change expected)_
## Options Considered
_disclaimer: these notes are verbatim from my research notes in [#4999]_.
### Create our own format for `INPUT_RECORD`s
* If we wanted to do this, then we'd probably want to have the Terminal only
Expand All @@ -398,7 +443,6 @@ _(no change expected)_
* We'd be defining our own VT sequences for these, which we've never done
before. This was _inevitable_, however, this is still the first time we'd be
doing this.
* Something about Microsoft extending well-defined protocols being bad 😆
* By having the Terminal send all input as _this protocol_, VT Input passthrough
to apps that want VT input won't work anymore for the Terminal. That's _okay_
Expand All @@ -409,11 +453,12 @@ _(no change expected)_
* Not terribly difficult to decode
* Unique from anything else we'd be processing, as it's an APC sequence
(`\x1b_`)
* From their docs: > All printable key presses without modifier keys are sent
* From their docs:
> All printable key presses without modifier keys are sent
just as in the normal mode. ... For non printable keys and key combinations
including one or more modifiers, an escape sequence encoding the key event is
sent
- I think like this. ASCII and other keyboard layout chars (things that would
- I think I like this. ASCII and other keyboard layout chars (things that would
hit `SendChar`) would still just come through as the normal char.
#### Cons:
Expand Down

1 comment on commit 70fbe9b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New misspellings found, please review:

  • Remoting
  • sco
To accept these changes, run the following commands 
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"atg BKMK consoleaccessibility shobjidl "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/70fbe9b92ac135364a3d968b9539886ea2c86c72.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"Remoting sco Shobjidl "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/70fbe9b92ac135364a3d968b9539886ea2c86c72.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Please sign in to comment.