Skip to content

Commit

Permalink
Fix unbinding keys in v0.10 (#4988)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
zadjii-msft authored Mar 18, 2020
1 parent c0d704e commit 8211ed9
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ std::vector<::TerminalApp::SettingsLoadWarnings> winrt::TerminalApp::implementat
warnings.insert(warnings.end(), parseWarnings.begin(), parseWarnings.end());

// if an arg parser was registered, but failed, bail
if (args == nullptr)
if (pfn && args == nullptr)
{
continue;
}
Expand Down

0 comments on commit 8211ed9

Please sign in to comment.