[PR #3658] [MERGED] Encapsulate dispatching ShortcutActions in it's own class #25462

Open
opened 2026-01-31 09:09:40 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/3658
Author: @zadjii-msft
Created: 11/21/2019
Status: Merged
Merged: 11/27/2019
Merged by: @zadjii-msft

Base: masterHead: dev/migrie/f/make-a-SAD


📝 Commits (3)

  • c5a5884 Move action handling to it's own class separate from AKB. This is the first checkbox in #3475
  • 4f826d3 clean up doc comments
  • ced790b Merge remote-tracking branch 'origin/master' into dev/migrie/f/make-a-SAD

📊 Changes

9 files changed (+427 additions, -327 deletions)

View changed files

📝 src/cascadia/TerminalApp/AppKeyBindings.cpp (+3 -179)
📝 src/cascadia/TerminalApp/AppKeyBindings.h (+5 -30)
📝 src/cascadia/TerminalApp/AppKeyBindings.idl (+2 -90)
src/cascadia/TerminalApp/ShortcutActionDispatch.cpp (+202 -0)
src/cascadia/TerminalApp/ShortcutActionDispatch.h (+61 -0)
src/cascadia/TerminalApp/ShortcutActionDispatch.idl (+103 -0)
📝 src/cascadia/TerminalApp/TerminalPage.cpp (+41 -28)
📝 src/cascadia/TerminalApp/TerminalPage.h (+3 -0)
📝 src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj (+7 -0)

📄 Description

Summary of the Pull Request

Moves all the code responsible for dispatching an ActionAndArgs to it's own class, ShortcutActionDispatch. Now, the AppKeyBindings just uses the single instance of a ShortcutActionDispatch that the TerminalPage owns to dispatch events, without the need to re-attach the event handlers every time we reload the settings.

References

This is something I originally did as a part of #2046.

I need this now for #607.

It's also a part of work for #3475

PR Checklist

  • This is a bullet point within #3475
  • I work here
  • Tests added/passed
  • [n/a] Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

With this change, we'll be able to have other things dispatch ShortcutActions easily, by constructing an ActionAndArgs and just passing it straight to the ShortcutActionDispatch.

Validation Steps Performed

Ran the Terminal, tried out some keybindings, namely Ctrl+c for copy when there is a selection, or send ^C when there isn't. That still works.

Reloading settings also 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/3658 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 11/21/2019 **Status:** ✅ Merged **Merged:** 11/27/2019 **Merged by:** [@zadjii-msft](https://github.com/zadjii-msft) **Base:** `master` ← **Head:** `dev/migrie/f/make-a-SAD` --- ### 📝 Commits (3) - [`c5a5884`](https://github.com/microsoft/terminal/commit/c5a5884bedcabc1b1af3a250dde31777eef5b096) Move action handling to it's own class separate from AKB. This is the first checkbox in #3475 - [`4f826d3`](https://github.com/microsoft/terminal/commit/4f826d3d73acf75b8ec59c44267420366eb2d788) clean up doc comments - [`ced790b`](https://github.com/microsoft/terminal/commit/ced790bd8e13da0713d7ed9ddfe9e19bdca3cc9f) Merge remote-tracking branch 'origin/master' into dev/migrie/f/make-a-SAD ### 📊 Changes **9 files changed** (+427 additions, -327 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/TerminalApp/AppKeyBindings.cpp` (+3 -179) 📝 `src/cascadia/TerminalApp/AppKeyBindings.h` (+5 -30) 📝 `src/cascadia/TerminalApp/AppKeyBindings.idl` (+2 -90) ➕ `src/cascadia/TerminalApp/ShortcutActionDispatch.cpp` (+202 -0) ➕ `src/cascadia/TerminalApp/ShortcutActionDispatch.h` (+61 -0) ➕ `src/cascadia/TerminalApp/ShortcutActionDispatch.idl` (+103 -0) 📝 `src/cascadia/TerminalApp/TerminalPage.cpp` (+41 -28) 📝 `src/cascadia/TerminalApp/TerminalPage.h` (+3 -0) 📝 `src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj` (+7 -0) </details> ### 📄 Description ## Summary of the Pull Request Moves all the code responsible for dispatching an `ActionAndArgs` to it's own class, `ShortcutActionDispatch`. Now, the `AppKeyBindings` just uses the single instance of a `ShortcutActionDispatch` that the `TerminalPage` owns to dispatch events, without the need to re-attach the event handlers every time we reload the settings. ## References This is something I originally did as a part of #2046. I need this now for #607. It's also a part of work for #3475 ## PR Checklist * [x] This is a bullet point within #3475 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments With this change, we'll be able to have other things dispatch `ShortcutAction`s easily, by constructing an `ActionAndArgs` and just passing it straight to the `ShortcutActionDispatch`. ## Validation Steps Performed Ran the Terminal, tried out some keybindings, namely <kbd>Ctrl+c</kbd> for copy when there is a selection, or send `^C` when there isn't. That still works. Reloading settings also 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:09:40 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#25462