starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #3391] [MERGED] Add support for arbitrary args in keybindings #25340

New Issue
Open
opened 2026-01-31 09:08:52 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:08:52 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/3391
Author: @zadjii-msft
Created: 10/31/2019
Status: ✅ Merged
Merged: 11/14/2019
Merged by: @zadjii-msft

Base: master ← Head: dev/migrie/f/1142-arbitrary-args

📝 Commits (10+)

  • 2bb524c this is a start, but there's a weird linker bug if I take the SetKeybinding(ShortcutAction, KeyChord) implementation out, which I don't totally understand
  • 8799451 a good old-fashioned clean will fix that right up
  • d93a0bc all these things work
  • e9a809b hey this actually functionally works
  • 26d9035 Mostly cleanup and completion of implementation
  • 4e43186 Hey I bet we could just make NewTab the handler for NewTabWithProfile
  • 9f426a7 Start writing tests for Keybinding args
  • 03a6486 Add tests
  • 7ce59ba Revert a bad sln change, and clean out dead code
  • 14f942b Merge branch 'master' into dev/migrie/f/1142-arbitrary-args

📊 Changes

18 files changed (+899 additions, -402 deletions)

View changed files

📝 doc/cascadia/profiles.schema.json (+167 -52)
📝 src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp (+182 -0)
➕ src/cascadia/TerminalApp/ActionAndArgs.cpp (+11 -0)
➕ src/cascadia/TerminalApp/ActionAndArgs.h (+18 -0)
📝 src/cascadia/TerminalApp/ActionArgs.cpp (+1 -1)
📝 src/cascadia/TerminalApp/ActionArgs.h (+112 -6)
📝 src/cascadia/TerminalApp/ActionArgs.idl (+7 -3)
📝 src/cascadia/TerminalApp/AppActionHandlers.cpp (+36 -14)
📝 src/cascadia/TerminalApp/AppKeyBindings.cpp (+45 -234)
📝 src/cascadia/TerminalApp/AppKeyBindings.h (+6 -5)
📝 src/cascadia/TerminalApp/AppKeyBindings.idl (+41 -29)
📝 src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp (+237 -36)
📝 src/cascadia/TerminalApp/CascadiaSettings.cpp (+4 -0)
📝 src/cascadia/TerminalApp/TerminalPage.cpp (+1 -2)
📝 src/cascadia/TerminalApp/TerminalPage.h (+1 -2)
📝 src/cascadia/TerminalApp/defaults.json (+18 -18)
📝 src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj (+6 -0)
📝 src/cascadia/ut_app/precomp.h (+6 -0)

📄 Description

Summary of the Pull Request

Enables the user to provide arbitrary argument values to shortcut actions through a new args member of keybindings. For some keybindings, like NewTabWithProfile<N>, we previously needed 9 different ShortcutActions, one for each value of Index. If a user wanted to have a NewTabWithProfile11 keybinding, that was simply impossible. Now that the args are in their own separate json object, each binding can accept any number of arbitrary argument values.

So instead of:

        { "command": "newTab", "keys": ["ctrl+shift+t"] },
        { "command": "newTabProfile0", "keys": ["ctrl+shift+1"] },
        { "command": "newTabProfile1", "keys": ["ctrl+shift+2"] },
        { "command": "newTabProfile2", "keys": ["ctrl+shift+3"] },
        { "command": "newTabProfile3", "keys": ["ctrl+shift+4"] },

We can now use:

        { "command": "newTab", "keys": ["ctrl+shift+t"] },
        { "command": { "action": "newTab", "index": 0 }, "keys": ["ctrl+shift+1"] },
        { "command": { "action": "newTab", "index": 1 }, "keys": ["ctrl+shift+2"] },
        { "command": { "action": "newTab", "index": 2 }, "keys": ["ctrl+shift+3"] },

Initially, this does seem more verbose. However, for cases where there are multiple args, or there's a large range of values for the args, this will quickly become a more powerful system of expressing keybindings.

The "legacy" keybindings are left in in this PR. They have helper methods to generate appropriate IActionArgs values. Prior to releasing 1.0, I think we should remove them, if only to remove some code bloat.

References

See the spec for more details.

This is part two of the implementation, part one was #2446

PR Checklist

Validation Steps Performed

  • Ran Tests
  • Removed the legacy keybindings from the defaults.json, everything still works
  • Tried leaving the legacy keybingings in my profiles.json, everything still works.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/3391 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 10/31/2019 **Status:** ✅ Merged **Merged:** 11/14/2019 **Merged by:** [@zadjii-msft](https://github.com/zadjii-msft) **Base:** `master` ← **Head:** `dev/migrie/f/1142-arbitrary-args` --- ### 📝 Commits (10+) - [`2bb524c`](https://github.com/microsoft/terminal/commit/2bb524c9916ff36d749045ffba004b6d13121392) this is a start, but there's a weird linker bug if I take the SetKeybinding(ShortcutAction, KeyChord) implementation out, which I don't totally understand - [`8799451`](https://github.com/microsoft/terminal/commit/8799451f6d643437a200a0a90dc1b945d0f8f9e6) a good old-fashioned clean will fix that right up - [`d93a0bc`](https://github.com/microsoft/terminal/commit/d93a0bc87a758de4f292de9e9eb83f6189dff1ed) all these things work - [`e9a809b`](https://github.com/microsoft/terminal/commit/e9a809b0f0c5ff705de17bf62c869689c4f016e0) hey this actually _functionally_ works - [`26d9035`](https://github.com/microsoft/terminal/commit/26d90359eaee83f768a2f67b08d0c68a2cb280db) Mostly cleanup and completion of implementation - [`4e43186`](https://github.com/microsoft/terminal/commit/4e43186ceee70b464ca64626c14133d681f92013) Hey I bet we could just make NewTab the handler for NewTabWithProfile - [`9f426a7`](https://github.com/microsoft/terminal/commit/9f426a76ef1643a7bbb3a2c97435dd4fa4fdcd51) Start writing tests for Keybinding args - [`03a6486`](https://github.com/microsoft/terminal/commit/03a648635c1f87a302134096499c98864c874e8f) Add tests - [`7ce59ba`](https://github.com/microsoft/terminal/commit/7ce59bafbb00f66b297a5c386ffc9134a6cb5003) Revert a bad sln change, and clean out dead code - [`14f942b`](https://github.com/microsoft/terminal/commit/14f942bb3a311e54aa537e8c24b83b213a327ffc) Merge branch 'master' into dev/migrie/f/1142-arbitrary-args ### 📊 Changes **18 files changed** (+899 additions, -402 deletions) <details> <summary>View changed files</summary> 📝 `doc/cascadia/profiles.schema.json` (+167 -52) 📝 `src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp` (+182 -0) ➕ `src/cascadia/TerminalApp/ActionAndArgs.cpp` (+11 -0) ➕ `src/cascadia/TerminalApp/ActionAndArgs.h` (+18 -0) 📝 `src/cascadia/TerminalApp/ActionArgs.cpp` (+1 -1) 📝 `src/cascadia/TerminalApp/ActionArgs.h` (+112 -6) 📝 `src/cascadia/TerminalApp/ActionArgs.idl` (+7 -3) 📝 `src/cascadia/TerminalApp/AppActionHandlers.cpp` (+36 -14) 📝 `src/cascadia/TerminalApp/AppKeyBindings.cpp` (+45 -234) 📝 `src/cascadia/TerminalApp/AppKeyBindings.h` (+6 -5) 📝 `src/cascadia/TerminalApp/AppKeyBindings.idl` (+41 -29) 📝 `src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp` (+237 -36) 📝 `src/cascadia/TerminalApp/CascadiaSettings.cpp` (+4 -0) 📝 `src/cascadia/TerminalApp/TerminalPage.cpp` (+1 -2) 📝 `src/cascadia/TerminalApp/TerminalPage.h` (+1 -2) 📝 `src/cascadia/TerminalApp/defaults.json` (+18 -18) 📝 `src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj` (+6 -0) 📝 `src/cascadia/ut_app/precomp.h` (+6 -0) </details> ### 📄 Description ## Summary of the Pull Request Enables the user to provide arbitrary argument values to shortcut actions through a new `args` member of keybindings. For some keybindings, like `NewTabWithProfile<N>`, we previously needed 9 different `ShortcutAction`s, one for each value of `Index`. If a user wanted to have a `NewTabWithProfile11` keybinding, that was simply impossible. Now that the args are in their own separate json object, each binding can accept any number of arbitrary argument values. So instead of: ```json { "command": "newTab", "keys": ["ctrl+shift+t"] }, { "command": "newTabProfile0", "keys": ["ctrl+shift+1"] }, { "command": "newTabProfile1", "keys": ["ctrl+shift+2"] }, { "command": "newTabProfile2", "keys": ["ctrl+shift+3"] }, { "command": "newTabProfile3", "keys": ["ctrl+shift+4"] }, ``` We can now use: ```json { "command": "newTab", "keys": ["ctrl+shift+t"] }, { "command": { "action": "newTab", "index": 0 }, "keys": ["ctrl+shift+1"] }, { "command": { "action": "newTab", "index": 1 }, "keys": ["ctrl+shift+2"] }, { "command": { "action": "newTab", "index": 2 }, "keys": ["ctrl+shift+3"] }, ``` Initially, this does seem more verbose. However, for cases where there are multiple args, or there's a large range of values for the args, this will quickly become a more powerful system of expressing keybindings. The "legacy" keybindings are _left in_ in this PR. They have helper methods to generate appropriate `IActionArgs` values. Prior to releasing 1.0, I think we should remove them, if only to remove some code bloat. ## References See [the spec](https://github.com/microsoft/terminal/blob/master/doc/specs/%231142%20-%20Keybinding%20Arguments.md) for more details. This is part two of the implementation, part one was #2446 ## PR Checklist * [x] Closes #1142 * [x] I work here * [x] Tests added/passed * [x] Schema updated ## Validation Steps Performed * Ran Tests * Removed the legacy keybindings from the `defaults.json`, everything still works * Tried leaving the legacy keybingings in my `profiles.json`, everything still works. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:08:52 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#25340
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?