Removed shortcuts for starting new sessions is not reflected in new-session dropdown #18401

Closed
opened 2026-01-31 06:12:45 +00:00 by claunia · 4 comments
Owner

Originally created by @vermiculus on GitHub (Sep 8, 2022).

Windows Terminal version

1.14.2281.0

Windows build number

10.0.19044.0

Other Software

No response

Steps to reproduce

  1. Have at least four (or some number of) profiles configured.
  2. Remove default keybindings like ctrl+shift+4 and make sure customizations are applied.
  3. Click dropdown to launch a new shell.

Expected Behavior

I would expect to not see ctrl+shift+4 to be advertised as bound.

Actual Behavior

You will see ctrl+shift+4 still advertised as bound.

image

Keybindings were removed from the right-hand side and are still visible in the drop-down. The keybindings don't work, as expected, but they are still being advertised.

Originally created by @vermiculus on GitHub (Sep 8, 2022). ### Windows Terminal version 1.14.2281.0 ### Windows build number 10.0.19044.0 ### Other Software _No response_ ### Steps to reproduce 1. Have at least four (or some number of) profiles configured. 1. Remove default keybindings like `ctrl+shift+4` and make sure customizations are applied. 2. Click dropdown to launch a new shell. ### Expected Behavior I would expect to not see `ctrl+shift+4` to be advertised as bound. ### Actual Behavior You will see `ctrl+shift+4` still advertised as bound. ![image](https://user-images.githubusercontent.com/2082195/189015738-6727dd63-1044-40be-a429-27ff8966e0de.png) Keybindings were removed from the right-hand side and are still visible in the drop-down. The keybindings don't work, as expected, but they are still being advertised.
Author
Owner

@DHowett commented on GitHub (Sep 8, 2022):

Thanks for the report!

@carlos-zamora, this is probably a layering/masking issue. The shortcuts exist on the lower layer (defaults.json) but have been unbound on the upper layer (settings.json).

@DHowett commented on GitHub (Sep 8, 2022): Thanks for the report! @carlos-zamora, this is probably a layering/masking issue. The shortcuts exist on the lower layer (defaults.json) but have been unbound on the upper layer (settings.json).
Author
Owner

@zadjii-msft commented on GitHub (Sep 8, 2022):

Shockingly, this isn't a test for this case.

    void KeyBindingsTests::TestUnbindReverseLookup()
    {
        Log::Comment(L"TODO!");

        // Wrap the first one in `R"!(...)!"` because it has `()` internally.
        const std::string bindings0String{ R"!([ { "command": { "action": "newTab", "index": 0 }, "keys":"ctrl+a" } ])!" };
        const std::string bindings1String{ R"([ { "keys": "ctrl+a", "command": null } ])" };

        const auto bindings0Json = VerifyParseSucceeded(bindings0String);
        const auto bindings1Json = VerifyParseSucceeded(bindings1String);

        auto actionMap = winrt::make_self<implementation::ActionMap>();
        VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size());

        actionMap->LayerJson(bindings0Json);
        VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size());

        NewTerminalArgs newTerminalArgs{ 0 };
        NewTabArgs newTabArgs{ newTerminalArgs };
        {
            auto keyChord{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) };
            VERIFY_IS_NOT_NULL(keyChord);
        }

        actionMap->LayerJson(bindings1Json);

        {
            auto keyChord{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) };
            VERIFY_IS_NULL(keyChord);
        }
    }

This actually does pass today on main, so there's gotta be some parenting trickiness that causes this to fail

@zadjii-msft commented on GitHub (Sep 8, 2022): Shockingly, this _isn't_ a test for this case. ```c++ void KeyBindingsTests::TestUnbindReverseLookup() { Log::Comment(L"TODO!"); // Wrap the first one in `R"!(...)!"` because it has `()` internally. const std::string bindings0String{ R"!([ { "command": { "action": "newTab", "index": 0 }, "keys":"ctrl+a" } ])!" }; const std::string bindings1String{ R"([ { "keys": "ctrl+a", "command": null } ])" }; const auto bindings0Json = VerifyParseSucceeded(bindings0String); const auto bindings1Json = VerifyParseSucceeded(bindings1String); auto actionMap = winrt::make_self<implementation::ActionMap>(); VERIFY_ARE_EQUAL(0u, actionMap->_KeyMap.size()); actionMap->LayerJson(bindings0Json); VERIFY_ARE_EQUAL(1u, actionMap->_KeyMap.size()); NewTerminalArgs newTerminalArgs{ 0 }; NewTabArgs newTabArgs{ newTerminalArgs }; { auto keyChord{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) }; VERIFY_IS_NOT_NULL(keyChord); } actionMap->LayerJson(bindings1Json); { auto keyChord{ actionMap->GetKeyBindingForAction(ShortcutAction::NewTab, newTabArgs) }; VERIFY_IS_NULL(keyChord); } } ``` This actually does pass today on main, so there's gotta be some parenting trickiness that causes this to fail
Author
Owner

@zadjii-msft commented on GitHub (Sep 8, 2022):

This is a test however: dev/migrie/b/13943-a-test-for-this

@zadjii-msft commented on GitHub (Sep 8, 2022): _This_ is a test however: [`dev/migrie/b/13943-a-test-for-this`](https://github.com/microsoft/terminal/compare/dev/migrie/b/13943-a-test-for-this)
Author
Owner

@zadjii-msft commented on GitHub (May 24, 2023):

ignore me, just adding some keywords: new tab key keys shortcut shortcuts menu bindings

@zadjii-msft commented on GitHub (May 24, 2023): ignore me, just adding some keywords: new tab key keys shortcut shortcuts menu bindings
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#18401