Remove redundant settings for transient menu

[scriptmode]

Often you wanna use the keys of a transient menu also as it's starting-point.
But at the moment this means you need to duplicate the menu for every used key.

Example of how it is now:

        {
          "key": "j",
          "name": "Move lines down",
          "type": "transient",
          "command": "editor.action.moveLinesDownAction",
          "bindings": [
            {
              "key": "j",
              "name": "Move lines down",
              "type": "command",
              "command": "editor.action.moveLinesDownAction"
            },
            {
              "key": "k",
              "name": "Move lines up",
              "type": "command",
              "command": "editor.action.moveLinesUpAction"
            }
          ]
        },
        {
          "key": "k",
          "name": "Move lines up",
          "type": "transient",
          "command": "editor.action.moveLinesUpAction",
          "bindings": [
            {
              "key": "j",
              "name": "Move lines down",
              "type": "command",
              "command": "editor.action.moveLinesDownAction"
            },
            {
              "key": "k",
              "name": "Move lines up",
              "type": "command",
              "command": "editor.action.moveLinesUpAction"
            }
          ]
        }

A better solution would be to have a "headless" transient, which automatically takes it's startingkeys from the menu itself.

        {
          "type": "transient",
          "bindings": [
            {
              "key": "J",
              "name": "Move lines down",
              "type": "command",
              "command": "editor.action.moveLinesDownAction"
            },
            {
              "key": "K",
              "name": "Move lines up",
              "type": "command",
              "command": "editor.action.moveLinesUpAction"
            }
          ]
        },

Or define a number of keys from which it will take the commands to the upper level,
while it leaves the remaining commands for the transient menu.

        {
          "keys": ["J", "K"]      <---- New Setting
          "type": "transient",
          "bindings": [
            {
              "key": "J",
              "name": "Move lines down",
              "type": "command",
              "command": "editor.action.moveLinesDownAction"
            },
            {
              "key": "K",
              "name": "Move lines up",
              "type": "command",
              "command": "editor.action.moveLinesUpAction"
            },
            {
              "key": "x",
              "name": "Do something else",
              "type": "command",
              "command": "some.action"
            }
          ]
        }

[stevenguh]

Although the current transient can be redundant than needed, it is explicit to handle two cases.

1. Execute a command before entering the menu (e.g. the move line you mentioned above)
2. Just entering the transient menu without command execution (e.g. zoom transient)

The proposed solutions you shown above will only solve the first case where the entering key is also executing the same command the same key inside of the transient. Also, not to mentioned that implementation the proposed solutions will make overrides more complicated (introduction of implicit keys inside of the transient menu and json key "keys" collision with the overrides)

I would love to explore a solution that can solve this that's not overtly complicated just to remove verbosity.

[MarcoIeni]

We could define a "whichkey.bindingsGroup" in the package.json file where we put reusable bindings sub menus.

{
    "whichkey.bindingsGroup": {
        "type": "array",
        "markdownDescription": "Reusable bindings of the which key menu",
        "default": [
            {
                "id": "moveLines",
                "bindings": [
                    {
                        "key": "j",
                        "name": "Move lines down",
                        "type": "command",
                        "command": "editor.action.moveLinesDownAction"
                    },
                    {
                        "key": "k",
                        "name": "Move lines up",
                        "type": "command",
                        "command": "editor.action.moveLinesUpAction"
                    }
                ]
            }
        ]
    },
    "whichkey.bindings": {
        "type": "array",
        "markdownDescription": "The bindings of the which key menu",
        "default": [
            {
                "key": "j",
                "name": "Move lines down",
                "type": "transient",
                "command": "editor.action.moveLinesDownAction",
                "bindings": "moveLines"
            },
            {
                "key": "k",
                "name": "Move lines up",
                "type": "transient",
                "command": "editor.action.moveLinesUpAction",
                "bindings": "moveLines"
            }
        ]
    }
}

This will work in other use cases, too. For instance, you will be able to create aliases (SPC w and SPC W could refer to the "window" bindings group for example).

[stevenguh]

This is another interesting proposal. I like the id reference system; however, it also means whichkey.register will need to add one extra field to point to the "reference" config like

commands.executeCommand("whichkey.register", {
  bindings: ["myExtension", "bindings"],
  overrides: ["myExtension", "bindingOveArrides"],
  groups: ["myExtension", "bindingGroups"],
  title: "My menu"
});

And for the show directly option, we will not able to handle if the bindings contains any ids (Probably be fine, as long as we documented it well?). In this proposed solutions, the overrides will be simpler to manage and potentially can use the grouping to break off parts of the binding not just transient into its own "group/reference". In a sense, that reminds me of #9 where it was proposed to read the config from a file, and potentially break off parts of the config into its own file. (maybe we should also think what might affect #9, and vice versa)

Interestingly, both approaches have its own pros and cons. I like the id approach a little more because it works well-ish with the existing system. I am still not entirely convinced that the added complexity is worth it in order to remove some verbosities of a handful of redundant transient at least atm. Also, adding more complexity to the existing config would force us to figure out the a strategy for unit testing sooner. However, I would say other enhancements like conditional would have a higher priority in terms getting implemented.

[MarcoIeni]

However, I would say other enhancements like conditional would have a higher priority in terms getting implemented.

Maybe in this issue we can decide the syntax and the approach.
After, we can open a new issue where we formalize those and we explain which parts of the code have to be edited.

If the "conditional" feature is independent from this one, maybe someone else could work on this. Otherwise we could simply leave the issue pending until "conditional" is implemented.

[stevenguh]

It's probably good to keep discussing syntax here. Out of all the the proposals, they all add complexity into the current system. The original proposals are somewhat with a smaller scope, and have impact on the overrides system. The id system is nice, but requires a lot of rework and needs to handle recursion of id, and is more complex than the original proposal.

Right now, I am putting on my KISS hat to keep the redundancy and verbosity for now. Hope we can figure out some less complex solution to this redundancy issue.

[stevenguh]

Released v0.9.0 finally! Closing this :)