Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the win key in keybindings #9783

Merged
5 commits merged into from
Apr 15, 2021
Merged

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

Does what it says on the can. People can now use win in a keybinding to
indicate that the chord needs win.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

For the record, I hate this. But it's great for quake mode, so meh. There's
shockingly more win keys claimed then you think - many more than the shortcut
guide even shows.

  • win+b: Focus the tray?
  • win+t: Focus the taskbar
  • win+p: Project...
  • win+c: The powertoys color picker
  • win+v: cloud clipboard

So the list of valid combos is vanishingly small. It's all about that win+~

Validation Steps Performed

Bound

        { "keys": [ "win+`" ], "command": "commandPalette" },

and yea, it works as expected

  For the record, I hate this. But it's great for quake mode, so _meh_. There's
  shockingly more win keys claimed then you think - many more than the shortcut
  guide even shows.
@ghost ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Apr 12, 2021
@github-actions
Copy link

Misspellings found, please review:

  • valuse
To accept these changes, run the following commands from this repository on this branch
pushd $(git rev-parse --show-toplevel)
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"aef aspnet boostorg BSODs Cac COINIT dahall DEFAPP DEFCON fde fea fmtlib HPCON isocpp mintty msvcrtd NVDA pinam QOL remoting Unk unte vcrt what3words xamarin "');
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)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/b310b1cffc5df62f652ab1781d282ccfac3bb050.txt";
use File::Path qw(make_path);
make_path ".github/actions/spelling/expect";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"cac coinit defapp hpcon MSVCRTD Remoting unk valuse "');
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;'
popd
✏️ 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/spelling/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/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/spelling/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. 😉

🗜️ If you see a bunch of garbage and it relates to a binary-ish string, please add a file path to the .github/actions/spelling/excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@DHowett
Copy link
Member

DHowett commented Apr 12, 2021

it actually works??? this is great

@carlos-zamora
Copy link
Member

I'll let you/Michael figure out the regex thing. But don't forget to update the docs plz. We probably want some kind of warning in the docs too.

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Apr 13, 2021

TODO:

@Don-Vito
Copy link
Contributor

@zadjii-msft - will it affect #9163?

@zadjii-msft
Copy link
Member Author

@zadjii-msft - will it affect #9163?

Oh, this, might. I'll take a look. Thanks!

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

Marking req changes until the regex is sussed out.

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 14, 2021
@zadjii-msft
Copy link
Member Author

Works in the flyout:
image

and cmdpal:
image

And it doesn't dismiss the selection ✔️ (or rather, win+shift+s still works the same)

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 14, 2021
@zadjii-msft
Copy link
Member Author

dev/migrie/f/653-QUAKE-MODE is ready to send once this merges (QUAKE-MODE has some of this win key code that needs to get the updates from this branch).

@zadjii-msft zadjii-msft assigned miniksa and unassigned zadjii-msft Apr 14, 2021
@miniksa miniksa removed their assignment Apr 15, 2021
@miniksa miniksa added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 15, 2021
@ghost
Copy link

ghost commented Apr 15, 2021

Hello @miniksa!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit eddb99e into main Apr 15, 2021
@ghost ghost deleted the dev/migrie/f/win-in-keybindings branch April 15, 2021 16:52
mpela81 pushed a commit to mpela81/terminal that referenced this pull request Apr 17, 2021
## Summary of the Pull Request

Does what it says on the can. People can now use `win` in a keybinding to
indicate that the chord needs <kbd>win</kbd>.

## References
* Done for microsoft#653
* See also microsoft#8888

## PR Checklist
* [x] Closes microsoft#3184
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

For the record, I hate this. But it's great for quake mode, so _meh_. There's
shockingly more win keys claimed then you think - many more than the shortcut
guide even shows.

* `win+b`: Focus the tray?
* `win+t`: Focus the taskbar
* `win+p`: Project...
* `win+c`: The powertoys color picker
* `win+v`: cloud clipboard

So the list of valid combos is vanishingly small. It's all about that <kbd>win+~</kbd>

## Validation Steps Performed

Bound
```json
        { "keys": [ "win+`" ], "command": "commandPalette" },
```

and yea, it works as expected
@ghost
Copy link

ghost commented May 25, 2021

🎉Windows Terminal Preview v1.9.1445.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding support for win in key bindings?
5 participants