Allow ordinary menus #353

jwhitaker-gridcog · 2024-12-17T21:45:15Z

Heya, thanks for maintaining this.

When playing with Strackeror's great Helix mode contribution, I was having trouble getting a command-hx menu to feel right due to the single hot key system. E.g. distilled to a three item menu, "fmt / Format", "w / Write", and "wbc / Write and Close", w and wbc can't be distinguished in the current system.

As a proof of concept, I wondered if menus could have a menu_type: "hotkey" (current behaviour, and default) or alternatively a menu_type: "palette", which is just a stock-standard vscode quickpick. Palette menus don't have the hotkey system but can just use vscode's already great type/select system which I've been using locally quite happily.

Demo:

Kooha-2024-12-18-08-42-33.webm.mp4

From menus config:

    "dance.menus": {
        "leader-hx": {
            "title": "Space",
            "items": {
                "f": {
                    "text": "Open file picker",
                    "command": "workbench.action.quickOpen"
                }
            }
        },
        "command-hx": {
            "title": "Helix command mode",
            "menu_type": "palette",
            "items": {
                "fmt": {
                    "text": "format",
                    "command": "editor.action.formatDocument"
                },
                "w": {
                    "text": "write",
                    "command": "workbench.action.files.save"
                },
                // "wbc": {
                //     "text": "write-buffer-close",
                //     "command": "dance.run",
                //     "args": [
                //         {
                //             "commands": [
                //                 "workbench.action.files.save",
                //                 "workbench.action.closeeditor"
                //             ]
                //         }
                //     ]
                // }
            }
        }
    },

This probably isn't wired up the best way, but was hoping to get some feedback on if this would be accepted in principal or not before fiddling further.

Thanks!

Strackeror · 2025-01-10T12:54:50Z

Ooooh, that's a great way to do a simple command palette, I'm stealing this for my fork :)

71

Thanks for the contribution! From what I see it should be easy to merge.

package.build.ts

src/api/menu.ts

jwhitaker-gridcog · 2025-02-22T15:58:49Z

camel_case exorcized, description/help text added to jsonschema, and more efficient "what was picked?" implemented. thanks for review!

jwhitaker-gridcog · 2025-02-23T06:12:29Z

package.build.ts

+                  },
+                  {
+                    const: "hotkey",
+                    description: "A `hotkey` menu is used to give a menu and keybinds emulating Helix's multi-key Static Commands, e.g. [g]oto → [e]nd."


Ah this should be a kakoune example sorry, will fix up when I get a chance

71

Thanks! Feel free to take your time if you want to improve documentation or something.

71 · 2025-02-23T07:38:25Z

src/api/menu.ts

-  const choice = await promptOne(items, (quickPick) => quickPick.title = menu.title);
+
+  let choice: string | number;
+  if ((menu.menuType ?? 'hotkey') == 'hotkey') {


nit (sorry):

Suggested change

if ((menu.menuType ?? 'hotkey') == 'hotkey') {

if ((menu.menuType ?? 'hotkey') === 'hotkey') {

No stress, thanks for picking up.
A general note: you seem to put a lot of effort into making sure contributions follow good+consistent code style. This is fine but probably means thorough review takes a lot of your time!

Would you consider setting up a linter to automate this? Things like Biome are amazingly fast these days, and as a contributor it means I can check (and sometimes even autofix!) these things before you even need to read them.

That's a great point! I really need (and want) to set this up, but haven't done so yet. Nowadays I mostly work with Deno, so I'll need to either migrate existing lints to Deno or learn other linters and formatters. I similarly want to adopt a new code style (damn me, having to always "Save Without Formatting" in VS Code), but that requires applying it to the whole repo and breaking pending PRs.

71 · 2025-02-23T07:41:56Z

package.build.ts

+                    description: "A `hotkey` menu is used to give a menu and keybinds emulating Helix's multi-key Static Commands, e.g. [g]oto → [e]nd."
+                  }
+                ],
+                description: 'meny type test',


Please fix :)

Ahh oops! Soz

71 · 2025-02-23T07:43:43Z

package.build.ts

@@ -361,6 +361,20 @@ export const pkg = (modules: Builder.ParsedModule[]) => ({
              title: {
                type: "string",
              },
+              menuType: {
+                anyOf: [


anyOf is more of an "advanced" JSON Schema feature. In VS Code markdownEnumDescriptions is better supported.

jwhitaker-gridcog · 2025-02-23T12:33:27Z

should be good now, sorry I'm not sure if you want you or me to resolve the feedback so I'll leave to you :)

EDIT: following on from the linter discussion above, I ran eslint for the first time.. i hadn't done this, oops. This raised that the typescript satisfies operator I'm using isn't supported on the dance's Typescript version. I've hence included a commit to bump typescript to the latest. Same commit includes two small fixups - newer typescript gets stressed out by functions with mixed implicit and explicit returns. I can bump this off if requested, but new Typescript is probably desirable.

jwhitaker-gridcog · 2025-03-13T09:30:25Z

Is this one waiting on anything from me? Apart from maybe removing 'wip' from the title :)

71

Thanks!

I've hence included a commit to bump typescript to the latest

Sounds good, thanks.

Two small comments and then I'll approve.

71 · 2025-04-05T06:06:08Z

src/api/menu.ts

@@ -165,7 +187,7 @@ export async function showMenuAfterDelay(
      const pickedItem = menu.items[itemKeys],
            args = mergeArgs(pickedItem.args, additionalArgs);

-      return Context.WithoutActiveEditor.wrap(
+      await Context.WithoutActiveEditor.wrap(


This should be followed by a return statement (unless that's intentional?).

eep, great catch thanks.

71 · 2025-04-05T06:12:04Z

package.build.ts

@@ -361,6 +361,18 @@ export const pkg = (modules: Builder.ParsedModule[]) => ({
              title: {
                type: "string",
              },
+              menuType: {


Sorry I only think about this now, but what about type? menu.menuType: palette is a bit redundant.

Co-authored-by: Grégoire Geis <[email protected]>

jwhitaker-gridcog · 2025-04-09T22:37:08Z

fixed up, ta.
I've snuck on one more commit on the end - it hooks up autocomplete for commands when the user edits their menu configs. :) I can pop this off and PR separately if you'd prefer, but it's a trivial one-liner so in here for now :)

71 · 2025-04-11T22:10:06Z

@jwhitaker-gridcog Thank you!

it hooks up autocomplete for commands when the user edits their menu configs

Works great, thanks!

71 reviewed Feb 22, 2025

View reviewed changes

package.build.ts Outdated Show resolved Hide resolved

src/api/menu.ts Outdated Show resolved Hide resolved

src/api/menu.ts Outdated Show resolved Hide resolved

jwhitaker-gridcog requested a review from 71 February 22, 2025 15:57

jwhitaker-gridcog commented Feb 23, 2025

View reviewed changes

71 reviewed Feb 23, 2025

View reviewed changes

jwhitaker-gridcog force-pushed the jarrad/quickpick branch from 7ec1274 to f878e62 Compare February 23, 2025 12:32

jwhitaker-gridcog requested a review from 71 February 23, 2025 12:33

jwhitaker-gridcog changed the title ~~wip: allow ordinary menus~~ Allow ordinary menus Mar 13, 2025

71 reviewed Apr 5, 2025

View reviewed changes

jwhitaker-gridcog and others added 8 commits April 10, 2025 08:07

wip: allow ordinary menus

8438cc9

Apply suggestions from code review

7820c3e

Co-authored-by: Grégoire Geis <[email protected]>

more PR feedback

1f5d1af

PR feedback

711cefc

bump typescript to 5.7

c629812

eslint / format

70ee309

menu.menuType -> menu.type

4ecfcbd

fix bug from code review, thanks @71

3d5c57d

jwhitaker-gridcog force-pushed the jarrad/quickpick branch from c9b8a04 to 3d5c57d Compare April 9, 2025 22:07

use vscode schema to give autocomplete on configuring menus

1b93370

jwhitaker-gridcog requested a review from 71 April 9, 2025 22:37

71 approved these changes Apr 11, 2025

View reviewed changes

71 merged commit 4e3d259 into 71:master Apr 11, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow ordinary menus #353

Allow ordinary menus #353

jwhitaker-gridcog commented Dec 17, 2024 •

edited

Loading

Strackeror commented Jan 10, 2025

71 left a comment

jwhitaker-gridcog commented Feb 22, 2025

jwhitaker-gridcog Feb 23, 2025

71 left a comment

71 Feb 23, 2025

jwhitaker-gridcog Feb 23, 2025

71 Feb 23, 2025

71 Feb 23, 2025

jwhitaker-gridcog Feb 23, 2025

71 Feb 23, 2025

jwhitaker-gridcog commented Feb 23, 2025 •

edited

Loading

jwhitaker-gridcog commented Mar 13, 2025

71 left a comment

71 Apr 5, 2025

jwhitaker-gridcog Apr 9, 2025

71 Apr 5, 2025

jwhitaker-gridcog Apr 9, 2025

jwhitaker-gridcog commented Apr 9, 2025

71 commented Apr 11, 2025

	if ((menu.menuType ?? 'hotkey') == 'hotkey') {
	if ((menu.menuType ?? 'hotkey') === 'hotkey') {

Allow ordinary menus #353

Allow ordinary menus #353

Conversation

jwhitaker-gridcog commented Dec 17, 2024 • edited Loading

Strackeror commented Jan 10, 2025

71 left a comment

Choose a reason for hiding this comment

jwhitaker-gridcog commented Feb 22, 2025

Choose a reason for hiding this comment

71 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwhitaker-gridcog commented Feb 23, 2025 • edited Loading

jwhitaker-gridcog commented Mar 13, 2025

71 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwhitaker-gridcog commented Apr 9, 2025

71 commented Apr 11, 2025

jwhitaker-gridcog commented Dec 17, 2024 •

edited

Loading

jwhitaker-gridcog commented Feb 23, 2025 •

edited

Loading