[PR #4988] [MERGED] Fix unbinding keys in v0.10 #26061

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

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/4988
Author: @zadjii-msft
Created: 3/18/2020
Status: Merged
Merged: 3/18/2020
Merged by: @zadjii-msft

Base: masterHead: dev/migrie/b/3729-fix-unbinding-again


📝 Commits (1)

📊 Changes

1 file changed (+1 additions, -1 deletions)

View changed files

📝 src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp (+1 -1)

📄 Description

Summary of the Pull Request

We (the royal "we") broke key unbinding in #4746. We didn't run the local tests after this, which actually would have caught this. The comment even suggests what we should have done here. We need to make sure that when we bail, it's because there's a parsing function that returned nothing. null, "unbound", etc actually don't even have a parsing function at all, so they should just keep on keepin' on.

References

Source of this regression: #4746

PR Checklist

Detailed Description of the Pull Request / Additional comments

This is a great example of why your unittests should run in CI always

Validation Steps Performed

  • ran the tests
  • tested the following unbindings:
        { "command": null, "keys": [ "ctrl+shift+t" ] },
        { "command": "unbound", "keys": [ "ctrl+shift+t" ] },
        { "command": "null", "keys": [ "ctrl+shift+t" ] },

and they each individually worked.


🔄 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/4988 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 3/18/2020 **Status:** ✅ Merged **Merged:** 3/18/2020 **Merged by:** [@zadjii-msft](https://github.com/zadjii-msft) **Base:** `master` ← **Head:** `dev/migrie/b/3729-fix-unbinding-again` --- ### 📝 Commits (1) - [`8d25adf`](https://github.com/microsoft/terminal/commit/8d25adf7f10c3bb87dd1c3643e7aa6b47eb3b379) oof ### 📊 Changes **1 file changed** (+1 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp` (+1 -1) </details> ### 📄 Description ## Summary of the Pull Request We (the royal "we") broke key unbinding in #4746. We didn't run the local tests after this, which actually would have caught this. The comment even suggests what we should have done here. We need to make sure that when we bail, it's because there's a parsing function that returned nothing. `null`, `"unbound"`, etc actually don't even have a parsing function at all, so they should just keep on keepin' on. ## References Source of this regression: #4746 ## PR Checklist * [x] Closes #3729 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments This is a great example of why your unittests should run in CI always ## Validation Steps Performed * **ran the tests** * tested the following unbindings: ```json { "command": null, "keys": [ "ctrl+shift+t" ] }, { "command": "unbound", "keys": [ "ctrl+shift+t" ] }, { "command": "null", "keys": [ "ctrl+shift+t" ] }, ``` and they each individually worked. --- <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:13:41 +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#26061