Skip to content

Commit

Permalink
Add "monitor": "any"|"toCurrent"|"toMouse" setting to globalSummon (#…
Browse files Browse the repository at this point in the history
…10092)

#### ⚠️ this pr targets #9977

## Summary of the Pull Request

This adds support for part of the `monitor` property for `globalSummon`. It also goes a little off-spec:

```json
"monitor": "any"|"toCurrent"|"toMouse"
```

* `monitor`: This controls the monitor that the window will be summoned from/to
  - `"any"`: Summon the MRU window, regardless of which monitor it's currently on.
  - `"toCurrent"`/omitted: (_default_): Summon the MRU window **TO** the monitor with the current **foreground** window.
  - [**NEW**] `"toMouse"`: Summon the MRU window **TO** the monitor where the **mouse** cursor is.

When I was playing with this, It felt like `toMouse` was always what I wanted, not `toCurrent`. We can always just comment that out if we think that's contentious - I'm aware I didn't originally spec that.

## References
* Original thread: #653
* Spec: #9274 
* megathread: #8888

## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-60325291
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated 😢 

## Detailed Description of the Pull Request / Additional comments

I made `toMouse` the default because it felt better. fite-me.jpg 

## Validation Steps Performed
my ever evolving blob:

```jsonc
    { "keys": "ctrl+`", "command": { "action": "quakeMode" } },
    { "keys": "ctrl+1", "command": { "action": "globalSummon" } },
    // { "keys": "ctrl+2", "command": { "action": "globalSummon", "desktop": "toCurrent" } },
    // { "keys": "ctrl+2", "command": { "action": "globalSummon", "toggleVisibility": false } },
    // { "keys": "ctrl+2", "command": { "action": "globalSummon", "dropdownDuration": 2000 } },
    { "keys": "ctrl+2", "command": { "action": "globalSummon", "monitor": "any" } },
    // { "keys": "ctrl+3", "command": { "action": "globalSummon", "desktop": "onCurrent" } },
    { "keys": "ctrl+3", "command": { "action": "globalSummon", "monitor": "toMouse" } },
    // { "keys": "ctrl+4", "command": { "action": "globalSummon", "desktop": "any" } },
    { "keys": "ctrl+4", "command": { "action": "globalSummon", "monitor": "toMouse", "dropdownDuration": 500 } },
    { "keys": "ctrl+5", "command": { "action": "globalSummon", "dropdownDuration": 500 } },
```
  • Loading branch information
zadjii-msft authored May 17, 2021
1 parent 6e11780 commit 3866771
Show file tree
Hide file tree
Showing 9 changed files with 271 additions and 29 deletions.
46 changes: 37 additions & 9 deletions doc/specs/#653 - Quake Mode/#653 - Quake Mode.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
author: Mike Griese @zadjii-msft
created on: 2021-02-23
last updated: 2021-04-21
last updated: 2021-05-13
issue id: #653
---

Expand Down Expand Up @@ -132,7 +132,7 @@ the window. To try and satisfy all these scenarios, I'm proposing the following
two arguments to the `globalSummon` action:
```json
"monitor": "any"|"toCurrent"|"onCurrent"|int,
"monitor": "any"|"toCurrent"|"toMouse"|"onCurrent"|int,
"desktop": "any"|"toCurrent"|"onCurrent"
```
Expand All @@ -141,8 +141,10 @@ The way these settings can be combined is in a table below. As an overview:
* `monitor`: This controls the monitor that the window will be summoned from/to
- `"any"`: Summon the MRU window, regardless of which monitor it's currently
on.
- `"toCurrent"`/omitted: (_default_): Summon the MRU window **TO** the current
monitor.
- `"toCurrent"`/omitted: (_default_): Summon the MRU window **TO** the monitor
with the current **foreground** window.
- `"toMouse"`: Summon the MRU window **TO** the monitor where the **mouse**
cursor is.
- `"onCurrent"`: Summon the MRU window **ALREADY ON** the current monitor.
- `int`: Summon the MRU window for the given monitor (as identified by the
"Identify" displays feature in the OS settings)
Expand Down Expand Up @@ -193,16 +195,33 @@ Else:
</tr>
<!-- ----------------------------------------------------------------------- -->
<tr>
<td><code>"toCurrent"</code><br> Summon the MRU window TO the current monitor</td>
<td>Go to the desktop the window is on, move to this monitor</td>
<td>Move the window to this desktop, move to this monitor</td>
<td><code>"toCurrent"</code><br> Summon the MRU window TO the monitor with the foreground window</td>
<td>Go to the desktop the window is on, move to the monitor with the foreground window</td>
<td>Move the window to this desktop, move to the monitor with the foreground window</td>
<td>

If there isn't one on this desktop:
* create a new one (on the monitor with the foreground window)

Else:
* activate the one on this desktop, move to the monitor with the foreground window
</td>
</tr>
<!-- ----------------------------------------------------------------------- -->
<tr>
<td>
<code>"toMouse"</code>
<sup><a href="#footnote-2">[2]</a></sup> <br>
Summon the MRU window TO the monitor with the mouse</td>
<td>Go to the desktop the window is on, move to the monitor with the mouse</td>
<td>Move the window to this desktop, move to the monitor with the mouse</td>
<td>

If there isn't one on this desktop:
* create a new one (on this monitor)
* create a new one (on the monitor with the mouse)

Else:
* activate the one on this desktop, move to this window
* activate the one on this desktop, move to the monitor with the mouse
</td>
</tr>
<!-- ----------------------------------------------------------------------- -->
Expand Down Expand Up @@ -673,6 +692,8 @@ aren't already included in this spec.
```
That would allow the user some further customizations on the quake mode
behaviors.
- This was later converted to the idea in [#9992] - Add per-window-name global
settings
* Another proposed idea was a simplification of some of the summoning modes. `{
"monitor": "any", "desktop": "any" }` is a little long, and maybe not the most
apparent naming. Perhaps we could add another property like `summonMode` that
Expand All @@ -698,9 +719,16 @@ windows. Once [#766] lands, this will give us a chance to persist the state of
_all_ open windows. This will allow us to re-open with all the user's windows,
not just the one that happened to be closed last.

<a name="footnote-2"><a>[2]: **Addenda, May 2021**: In the course of
implementation, it became apparent that there's an important UX difference
between summoning _to the monitor with the cursor_ vs _to the monitor with the
foreground window_. `"monitor": "toMouse"` was added as an option, to allow the
user to differentiate between the two behaviors.

[#653]: https://github.com/microsoft/terminal/issues/653
[#766]: https://github.com/microsoft/terminal/issues/766
[#5727]: https://github.com/microsoft/terminal/issues/5727
[#9992]: https://github.com/microsoft/terminal/issues/9992

[Process Model 2.0 Spec]: https://github.com/microsoft/terminal/blob/main/doc/specs/%235000%20-%20Process%20Model%202.0/%235000%20-%20Process%20Model%202.0.md
[Quake 3 sample]: https://youtu.be/ZmR6HQbuHPA?t=27
Expand Down
12 changes: 10 additions & 2 deletions src/cascadia/Remoting/Peasant.idl
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,21 @@ namespace Microsoft.Terminal.Remoting
Windows.Foundation.DateTime ActivatedTime { get; };
};


enum MonitorBehavior
{
InPlace,
ToCurrent,
ToMouse,
};


[default_interface] runtimeclass SummonWindowBehavior {
SummonWindowBehavior();
Boolean MoveToCurrentDesktop;
Boolean ToggleVisibility;
UInt32 DropdownDuration;
// Other options:
// * CurrentMonitor
MonitorBehavior ToMonitor;
}

interface IPeasant
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/Remoting/SummonWindowBehavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
WINRT_PROPERTY(bool, MoveToCurrentDesktop, true);
WINRT_PROPERTY(bool, ToggleVisibility, true);
WINRT_PROPERTY(uint32_t, DropdownDuration, 0);
WINRT_PROPERTY(Remoting::MonitorBehavior, ToMonitor, Remoting::MonitorBehavior::ToCurrent);

public:
SummonWindowBehavior(const Remoting::SummonWindowBehavior& other) :
_MoveToCurrentDesktop{ other.MoveToCurrentDesktop() },
_ToMonitor{ other.ToMonitor() },
_DropdownDuration{ other.DropdownDuration() },
_ToggleVisibility{ other.ToggleVisibility() } {};
};
Expand Down
7 changes: 6 additions & 1 deletion src/cascadia/TerminalSettingsModel/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1158,11 +1158,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
GlobalSummonArgs() = default;
WINRT_PROPERTY(winrt::hstring, Name, L"");
WINRT_PROPERTY(Model::DesktopBehavior, Desktop, Model::DesktopBehavior::ToCurrent);
WINRT_PROPERTY(Model::MonitorBehavior, Monitor, Model::MonitorBehavior::ToMouse);
WINRT_PROPERTY(bool, ToggleVisibility, true);
WINRT_PROPERTY(uint32_t, DropdownDuration, 0);

static constexpr std::string_view NameKey{ "name" };
static constexpr std::string_view DesktopKey{ "desktop" };
static constexpr std::string_view MonitorKey{ "monitor" };
static constexpr std::string_view ToggleVisibilityKey{ "toggleVisibility" };
static constexpr std::string_view DropdownDurationKey{ "dropdownDuration" };

Expand All @@ -1175,6 +1177,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
return otherAsUs->_Name == _Name &&
otherAsUs->_Desktop == _Desktop &&
otherAsUs->_Monitor == _Monitor &&
otherAsUs->_DropdownDuration == _DropdownDuration &&
otherAsUs->_ToggleVisibility == _ToggleVisibility;
}
Expand All @@ -1186,6 +1189,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
auto args = winrt::make_self<GlobalSummonArgs>();
JsonUtils::GetValueForKey(json, NameKey, args->_Name);
JsonUtils::GetValueForKey(json, DesktopKey, args->_Desktop);
JsonUtils::GetValueForKey(json, MonitorKey, args->_Monitor);
JsonUtils::GetValueForKey(json, DropdownDurationKey, args->_DropdownDuration);
JsonUtils::GetValueForKey(json, ToggleVisibilityKey, args->_ToggleVisibility);
return { *args, {} };
Expand All @@ -1195,6 +1199,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
auto copy{ winrt::make_self<GlobalSummonArgs>() };
copy->_Name = _Name;
copy->_Desktop = _Desktop;
copy->_Monitor = _Monitor;
copy->_DropdownDuration = _DropdownDuration;
copy->_ToggleVisibility = _ToggleVisibility;
return *copy;
Expand All @@ -1213,7 +1218,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
}
size_t Hash() const
{
return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(_Name, _Desktop, _ToggleVisibility);
return ::Microsoft::Terminal::Settings::Model::HashUtils::HashProperty(_Name, _Desktop, _Monitor, _DropdownDuration, _ToggleVisibility);
}
};

Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ namespace Microsoft.Terminal.Settings.Model
OnCurrent,
};

enum MonitorBehavior
{
Any,
ToCurrent,
ToMouse,
};

[default_interface] runtimeclass NewTerminalArgs {
NewTerminalArgs();
NewTerminalArgs(Int32 profileIndex);
Expand Down Expand Up @@ -266,6 +273,7 @@ namespace Microsoft.Terminal.Settings.Model
{
String Name { get; };
DesktopBehavior Desktop { get; };
MonitorBehavior Monitor { get; };
Boolean ToggleVisibility { get; };
UInt32 DropdownDuration { get; };
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,12 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::DesktopBehavior)
pair_type{ "onCurrent", ValueType::OnCurrent },
};
};

JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::MonitorBehavior)
{
JSON_MAPPINGS(3) = {
pair_type{ "any", ValueType::Any },
pair_type{ "toCurrent", ValueType::ToCurrent },
pair_type{ "toMouse", ValueType::ToMouse },
};
};
21 changes: 19 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,9 +589,13 @@ bool AppHost::HasWindow()
void AppHost::_DispatchCommandline(winrt::Windows::Foundation::IInspectable /*sender*/,
Remoting::CommandlineArgs args)
{
const Remoting::SummonWindowBehavior summonArgs{};
summonArgs.MoveToCurrentDesktop(false);
summonArgs.DropdownDuration(0);
summonArgs.ToMonitor(Remoting::MonitorBehavior::InPlace);
// Summon the window whenever we dispatch a commandline to it. This will
// make it obvious when a new tab/pane is created in a window.
_window->SummonWindow(false, 0);
_window->SummonWindow(summonArgs);
_logic.ExecuteCommandline(args.Commandline(), args.CurrentDirectory());
}

Expand Down Expand Up @@ -695,6 +699,19 @@ void AppHost::_GlobalHotkeyPressed(const long hotkeyIndex)
args.SummonBehavior().ToggleVisibility(summonArgs.ToggleVisibility());
args.SummonBehavior().DropdownDuration(summonArgs.DropdownDuration());

switch (summonArgs.Monitor())
{
case Settings::Model::MonitorBehavior::Any:
args.SummonBehavior().ToMonitor(Remoting::MonitorBehavior::InPlace);
break;
case Settings::Model::MonitorBehavior::ToCurrent:
args.SummonBehavior().ToMonitor(Remoting::MonitorBehavior::ToCurrent);
break;
case Settings::Model::MonitorBehavior::ToMouse:
args.SummonBehavior().ToMonitor(Remoting::MonitorBehavior::ToMouse);
break;
}

_windowManager.SummonWindow(args);
if (args.FoundMatch())
{
Expand Down Expand Up @@ -776,7 +793,7 @@ bool AppHost::_LazyLoadDesktopManager()
void AppHost::_HandleSummon(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const Remoting::SummonWindowBehavior& args)
{
_window->SummonWindow(args.ToggleVisibility(), args.DropdownDuration());
_window->SummonWindow(args);

if (args != nullptr && args.MoveToCurrentDesktop())
{
Expand Down
Loading

0 comments on commit 3866771

Please sign in to comment.