Terminal: Terminal doesn't support serializing multiple bindings for the same action

[trajano]

AppKeyBindingsSerialization::ToJson iterates over _actions_ and serializes down the first key it can find.

---

Environment

Microsoft Windows [Version 10.0.18947.1000]
Windows Terminal latest store version

Steps to reproduce

Haven't found a pattern yet.

Expected behavior

My shortcuts which I have defined in https://gist.github.com/trajano/24f4edccd9a997fad8b4de29ea252cc8 should be kept

Actual behavior

My shortcuts for copy (ctrl+insert) and paste (shift+insert) keep on disappearing.

[DHowett-MSFT]

This may just be our key deserializer not being broad enough. When it finds a key it cannot read, it discards it. Can you try ins instead of insert?

[trajano]

ctrl+insert worked correctly and ctrl+ins is not triggering the copy command.

[DHowett-MSFT]

Sorry, I can't tell what you're reporting now. ctrl+insert worked correctly, but it is also being deleted from your profiles.json? And it's not working correctly?

[trajano]

The shortcuts that I have defined in the file (as shown in the gist) keep on getting removed and reverted to the original set of shortcuts.

Your suggestion was to use ctrl+ins rather than ctrl-insert but that does not even get recognized by Terminal.

[trajano]

It could be related to https://github.com/microsoft/terminal/issues/2411

[zadjii-msft]

Maybe the whole insert thing is a red herring. Let's narrow the problem down.

Can you change _any_ keybindings to a different keys value, and have the settings persist? For example, try changing closePane to:

            { "command" : "closePane", "keys" :  [ "ctrl+w" ] },

If that works, then try setting closePane to "ctrl+insert". Does that persist without reloading? If that persists correctly, does ctrl+insert work to close a tab?

If that works, then lets try setting copy to "ctrl+insert". Does that persist without reloading? If that persists correctly, does ctrl+insert work to copy?

[dsafa]

I believe it's because there are duplicate commands defined

{
    "command" : "copy",
    "keys" : 
    [
        "ctrl+shift+c"
    ]
},
{
    "command" : "copy",
    "keys" : 
    [
        "ctrl+insert"
    ]
},

Same for paste. You can test this by duplicating another shortcut and only one will remain.

I'm not sure if its documented if multiple bindings is supposed to be supported? (my searching is failing me). I took a look at AppKeyBindingsSerialization::ToJson and it only seems to iterate once and serialize the first keybinding for a given action from AppKeyBindings::GetKeyBinding. Edit: Nevermind, multiple bindings are supported, its just an issue with serializing them

[trajano]

if that's the case I may as well get rid of the default binding for now since I don't really use it. But I'd like the support for multiple because if other people use my keyboard and they're familiar with the default at least it won't shock them.

[DHowett-MSFT]

We definitely thought that multiple bindings for the same action would work. If that's really what's happening, I'm going to repurpose this issue for that.

[dsafa]

For clarity: The multiple shortcuts will work the first time terminal loads the settings with multiple bindings but not for subsequent restarts.
Heres an issue report for completeness

Environment

Windows version: 10.0.18362.0
Windows Terminal version (if applicable): all

Steps to reproduce

1. Add multiple shortcuts for the same command to profiles.json
   example 2 copy shortcuts:

{
    "command" : "copy",
    "keys" : 
    [
        "ctrl+shift+c"
    ]
},
{
    "command" : "copy",
    "keys" : 
    [
        "ctrl+insert"
    ]
}

1. Start/restart windows terminal.
2. Try the different key bindings to invoke the command

Expected behavior

All shortcuts defined for the command should work and persist over multiple restarts of terminal.

Actual behavior

The multiple shortcuts defined for the command will only work until the next restart. On subsequent runs, only one of the shortcuts for the command will remain in settings.json.

Details

When the settings are serialized (such as on startup after the settings are loaded), only one shortcut per command is serialized. So the next startup will only have one shortcut.
Relevant functions are AppKeyBindingsSerialization::ToJson and AppKeyBindings::GetKeyBinding.

[DHowett-MSFT]

Moving this out to backlog since we're not planning on serializing any settings until the UI lands.

[carlos-zamora]

Moving this out to backlog since we're not planning on serializing any settings until the UI lands.

Moving this into v2 milestone since settings UI is probably something we'll start looking at soon.