Add `globalSummon` action by zadjii-msft · Pull Request #9854 · microsoft/terminal

zadjii-msft · 2021-04-16T14:07:27Z

Summary of the Pull Request

Adds support for two new actions:

globalSummon, which can be used to activate a window using a global (READ: OS-level) hotkey.
- accepts an optional name argument. When provided, this will attempt to summon with the given name. When omitted, we'll try to summon the most recent window.
quakeMode which is globalSummon for the _quake window.

These actions are stored in the actions array, but are read by the WindowsTerminal level and bound to the OS in IslandWindow. The monarch registers for these keybindings with the OS. When one is pressed, the monarch will recieve a WM_HOTKEY message. It'll use that to look up the corresponding action args. It'll use those to try and simmon the right window.

References

[MEGATHREAD] Quake Mode & Global Summon #8888: Quake mode megathread
Spec for Quake Mode #9274: Spec (guys seriously i just need one more ✔️)
Make the window name _quake special #9785: The start of granting "_quake" super powers

PR Checklist

Closes Feature request: hot key drop down ala quake/guake/tilda #653 - I'm gonna say this closes it for now, though we have many follow-ups in [MEGATHREAD] Quake Mode & Global Summon #8888
I work here
Tests added/passed
Requires documentation to be updated - I'm explicitly not adding docs until this is more rounded out at the end of 1.9, because this area's going to get a lot of churn

Detailed Description of the Pull Request / Additional comments

I'd love to post screenshots and gifs of this all over twitter, but hey that's frowned upon

Validation Steps Performed

Validated that it works with win keys
Validated that it works without win keys
Validated that it hot-reloads
Validated that it moves to the new monarch
Validated that you can bind both globalSummon and quakeMode at the same time and do different things
Validated that you can bind globalSummon with a name and it creates that name if it doesn't already exist

…ompile a String[]

…then back!

…!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

…safely tear it down. It _seems_ like it.

….dll

…galactic-empire

…ndow should be. It just always says 0 for now, but in the future it could actually give us useful info.

(cherry picked from commit 9f9eacb)

carlos-zamora

Approving. Only thing is to create a better localized text when a name is provided.

Hoping this can merge in soon so I can propagate any necessary changes to my ActionMap PR #9621

carlos-zamora · 2021-04-21T17:39:28Z

src/cascadia/TerminalSettingsModel/ActionArgs.cpp

+        // "Summon the Terminal window, name:\"{_Name}\""
+        if (!_Name.empty())
+        {
+            ss << L", name: ";
+            ss << std::wstring_view(_Name);
+        }


I know we've gotten away with it before, but "name" isn't really localized. How about Summon the "<Name>" window

I'm just gonna punt this discussion to the next PR I've got queued up, because it's gonna become a quagmire of

Summon the Terminal window named "Foo" to the current desktop (toCurrent/toCurrent/"Foo")

Summon the Terminal window on the current desktop (onCurrent/toCurrent)

Summon the Terminal window (toCurrent/toCurrent)

Go to the Terminal window (any/toCurrent)

Summon the Terminal window named "Foo" to the current desktop, to the current monitor

Summon the Terminal window on the current desktop, to the current monitor

Summon the Terminal window on the current desktop, on the current monitor

Summon the Terminal window on the current monitor, to the current desktop (this one had to change order in English to make sense)

y i k e s

cinnamon-msft · 2021-04-22T01:18:30Z

To add some visuals, I made this gif of quake mode :)

…E-MODE

DHowett · 2021-04-28T20:22:10Z

Format the code -- DO NOT AUTOMERGE as it will destroy the open branches with stacked PRs.

zadjii-msft · 2021-04-28T20:24:41Z

Format the code -- DO NOT AUTOMERGE as it will destroy the open branches with stacked PRs.

can do. Once #9954 is approved, I could just merge top-down lol

…E-MODE

DHowett

Commenting now to see all my comments in one place to decide whether to request/approve.

DHowett · 2021-04-28T20:24:12Z

src/cascadia/TerminalApp/AppLogic.idl


        FindTargetWindowResult FindTargetWindow(String[] args);

+        Windows.Foundation.Collections.IMap<Microsoft.Terminal.Control.KeyChord, Microsoft.Terminal.Settings.Model.ActionAndArgs> GlobalHotkeys();


Technically a View

DHowett · 2021-04-28T20:25:59Z

src/cascadia/TerminalSettingsModel/KeyMapping.cpp

+
+        for (const auto& [k, v] : _keyShortcuts)
+        {
+            if (v.Action() == ShortcutAction::GlobalSummon)


discussion hook: This is crying out for Actions to have Scopes 😉

if only there was hours in the day to even imagine spec'ing that idea 😋

src/cascadia/TerminalSettingsModel/KeyMapping.cpp

src/cascadia/TerminalSettingsModel/KeyMapping.idl

DHowett · 2021-04-28T20:28:39Z

src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw

-  <!-- 
-    Microsoft ResX Schema 
-    
+  <!--


re-save this file in VS if you edited it with SUBL

DHowett · 2021-04-28T20:30:52Z

src/cascadia/WindowsTerminal/AppHost.cpp

+            else
+            {
+                // We should make the window ourselves.
+                _createNewTerminalWindow(summonArgs);


Worried about the proliferation of blocks of "run wt.exe with these args" code...

this is clever/cool though

src/cascadia/WindowsTerminal/AppHost.cpp

DHowett · 2021-04-28T20:34:35Z

Okay, my only NAK comment is the postprocessing one. If it's justifiable, I'm okay with it ... but it does put some smarts in the reader rather than the loader.

ghost · 2021-05-25T19:11:24Z

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

Handy links:

zadjii-msft added 30 commits December 16, 2020 07:59

Get all the projects created and hooked up to the sln

3bef7bb

Yank all the M/P files, this builds?!

1f52d35

Shockingly, this works, it works elevated, and it works unpackaged

5a9cdc8

This won't work, but I'm committing this becaus I finally got it to c…

36539cf

…ompile a String[]

whoop, we pass the commandline from the peasant, to the monarch, and …

27ace16

…then back!

HOLY SHIT I GOT THE COMANDLINE TO EXECUTE IN THE CURRENT WINDOW!!!!!!…

9a41647

…!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

add a note to future me

5cabcfb

This works as a unittest, but not a local test. That's batty

03bfc6e

this macro makes me feel dirty

590b9ff

LocalTests_Remoting -> UnitTests_Remoting

0579b24

finish renaming this test

a3faed6

Clean up a lot for review

9c6eac4

cleanup Peasant for review

0103331

Final cleanup for review

b4fe1bf

_final_ final cleanup for review

d08e65c

pr nits

e101efd

This seems to work to create a thread listening for the monarch, and …

e1402d8

…safely tear it down. It _seems_ like it.

it's hard to believe he's gone / were it so easy

f978a9c

Oh yea actually remove the thing

5939636

last commit before the holidays

c088895

Merge remote-tracking branch 'origin/main' into dev/migrie/f/remoting…

fa2df47

….dll

Enable audit mode

0f5c24f

nits

921d915

Merge branch 'dev/migrie/f/remoting.dll' into dev/migrie/f/the-first-…

00184e7

…galactic-empire

Ask the TerminalApp to parse the commandline, and tell us what the wi…

658db6b

…ndow should be. It just always says 0 for now, but in the future it could actually give us useful info.

Holy bajesus, this works like a charm

977db46

Add a note about commiting suicide

bcbef34

notes, comments, cleanup

813dbc6

activate windows

0f0df5e

add some tracelogging

9fc2f0e

zadjii-msft added 2 commits April 21, 2021 12:01

!! THIS NEEDS TO GO TO THE PARENT BRANCH !!

15a8a9c

(cherry picked from commit 9f9eacb)

replace todo with comment

8370789

carlos-zamora approved these changes Apr 21, 2021

View reviewed changes

carlos-zamora removed their assignment Apr 21, 2021

carlos-zamora added the Needs-Second It's a PR that needs another sign-off label Apr 21, 2021

ghost requested review from PankajBhojwani, leonMSFT and miniksa April 21, 2021 17:42

Merge remote-tracking branch 'origin/main' into dev/migrie/f/653-QUAK…

4eb1d3a

…E-MODE

ghost requested a review from lhecker April 23, 2021 11:03

zadjii-msft mentioned this pull request Apr 26, 2021

Add desktop param to globalSummon; set _quake = toCurrent #9954

Merged

4 tasks

Merge remote-tracking branch 'origin/main' into dev/migrie/f/653-QUAK…

643b860

…E-MODE

zadjii-msft mentioned this pull request Apr 28, 2021

When the window is summoned and is already active, minimize it. #9963

Merged

5 tasks

Merge remote-tracking branch 'origin/main' into dev/migrie/f/653-QUAK…

aec2561

…E-MODE

zadjii-msft mentioned this pull request Apr 28, 2021

Add property to control dropdown speed of global summon #9977

Merged

3 tasks

Merge remote-tracking branch 'origin/main' into dev/migrie/f/653-QUAK…

5940f05

…E-MODE

DHowett reviewed Apr 28, 2021

View reviewed changes

Address Dustin's PR comments

06cb41b

zadjii-msft requested a review from DHowett April 28, 2021 21:46

zadjii-msft assigned DHowett Apr 28, 2021

DHowett approved these changes Apr 28, 2021

View reviewed changes

DHowett merged commit d08271e into main Apr 28, 2021

DHowett deleted the dev/migrie/f/653-QUAKE-MODE branch April 28, 2021 22:13

ghost mentioned this pull request May 25, 2021

Feature request: hot key drop down ala quake/guake/tilda #653

Closed

stewSquared mentioned this pull request Oct 21, 2021

Windows terminal launch hot key microsoft/PowerToys#2629

Closed


		FindTargetWindowResult FindTargetWindow(String[] args);

		Windows.Foundation.Collections.IMap<Microsoft.Terminal.Control.KeyChord, Microsoft.Terminal.Settings.Model.ActionAndArgs> GlobalHotkeys();

Conversation

zadjii-msft commented Apr 16, 2021

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

carlos-zamora left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cinnamon-msft commented Apr 22, 2021

Uh oh!

DHowett commented Apr 28, 2021

Uh oh!

zadjii-msft commented Apr 28, 2021

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DHowett commented Apr 28, 2021

Uh oh!

ghost commented May 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants