Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended version of executeCommand() with prefixArgument #1146

Closed
whitphx opened this issue Dec 23, 2021 · 5 comments · Fixed by #1150
Closed

Extended version of executeCommand() with prefixArgument #1146

whitphx opened this issue Dec 23, 2021 · 5 comments · Fixed by #1150

Comments

@whitphx
Copy link
Owner

whitphx commented Dec 23, 2021

Ref: #1127 (comment)

To call arbitrary commands with prefix argument, this extension should provide its own executeCommand().

The command name is like emacs-mcx.executeCommandWithPrefixArgument()?

API design plan:

vscode.commands.executeCommand("emacs-mcx.executeCommand", { command: "<commandName>", args: {...} })
vscode.commands.executeCommand("emacs-mcx.executeCommand", ["<commandName>", {...} ])

will call <commandName> command with arguments { ...args, prefixArgument }.

The key name prefixArgument can be customizable with the third argument of emacs-mcx.executeCommand()

@whitphx
Copy link
Owner Author

whitphx commented Dec 29, 2021

@tshino Hi, I created a prototype of the command emacs-mcx.executeCommandWithPrefixArgument in #1150, though which you can call another command with the prefix argument.
For example, if you define the keybinding below,

      {
        "key": "ctrl+x e",
        "command": "emacs-mcx.executeCommandWithPrefixArgument",
        "args": {
          "command": "kb-macro.playback",
          "prefixArgumentKey": "repeat"
        }
      }

C-u C-x e will execute kb-macro.playback with the argument { repeat: 4 }.

Does it satisfy your needs? Can you give some comments, suggestions, or any ideas on it?

@tshino
Copy link

tshino commented Dec 29, 2021

@whitphx Thank you so much. The API design looks perfect for my use case!

But I found one problem with the implementation at 0d4db90.
I recorded the following sequence.

C-f C-n

And triggered playback of the sequence by C-u C-x e with the keybinding you described.
Then the cursor moved as if I typed:

C-f C-f C-f C-f C-n
C-f C-n
C-f C-n
C-f C-n

I think the prefix argument was applied twice.
One was to the proxy and the other was to the first call of emacs-mcx.forwardChar.

@whitphx
Copy link
Owner Author

whitphx commented Dec 30, 2021

@tshino Thank you, good catch!
The prefix argument has not been cleared before calling the target command with the current implementation.
I updated the PR, so can you please try it out again?

@tshino
Copy link

tshino commented Dec 30, 2021

@whitphx It works perfectly for me!

After this new command has been shipped successfully with your extension, I will update the keymap wrapper file on my extension to enable prefix-argument support.
To mitigate dependency risks, I will arrange the keybindings to use the emacs-mcx.prefixArgumentExists context to call the new emacs-mcx.executeCommandWithPrefixArgument command only when it is needed.

Thanks a lot!

@whitphx
Copy link
Owner Author

whitphx commented Dec 30, 2021

@tshino Thank you for the trial. I'm glad to hear it works good.

To mitigate dependency risks, I will arrange the keybindings to use the emacs-mcx.prefixArgumentExists context to call the new emacs-mcx.executeCommandWithPrefixArgument command only when it is needed.

It makes sense 👌

emacs-mcx.executeCommandWithPrefixArgument will be included in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants