Skip to content

Extensions: Command callback arguments are getting harder to deal with #25716

@eamodio

Description

@eamodio
  • VSCode Version: Code - Insiders 1.12.0-insider (0bec115, 2017-04-28T18:57:52.518Z)
  • OS Version: Windows_NT ia32 10.0.16184

In the docs registerCommand has callback args of args: any[], while registerTextEditorCommand as args of (textEditor: TextEditor, edit: TextEditorEdit, args: any[]

These, of course, are accurate, but don't mention that when a command is called from different contexts some extra arguments may be included at the start of the args array. OK, that isn't entirely true, it is mentioned in the note here but it also isn't 100% accurate -- as the inference can also happen from the command palette as well as possibly other cases.

Anyway, so say a command is triggered from file explorer item context menu, or an editor context menu -- that the first argument in the args: any[] is going to be a Uri, and now with the new SCM menus the command callback args can contain an arbitrary number of items (the SCM resources selected). And compound that with command execution via executeCommand as well as keybindings supporting argument passing -- its getting hard to deal with the differing args.

Sure, different commands can be registered for different contexts, but that doesn't feel right, especially with shortcut keys, command palette entries, etc (yes, these can be dealt with in most cases with when clauses, but that feels like extra and unneeded complexity -- at least imo).

Honestly I'm not really sure how to solve this, and maybe the recommended solution is to use different commands for different contexts, but maybe there is another way?

One thought is to somehow separate "system-supplied" arguments, vs "user-supplied", or have some sort of "context" parameter (at a known and stable position) so that the type of arguments could be inferred. Or maybe a combination of both, something like:

declare type CommandSource = 'executeCommand' | 'commandPalette' | 'explorer/context' | 'editor/context' | 'editor/title'; //  ... you get the picture

callback: (source: CommandSource, sourceArgs: any[], args: any[]) => any

// OR -- better yet imo

interface EditorCommandContext {
    textEditor: TextEditor;
    edit: TextEditorEdit;
    uri?: Uri;
}

interface ExplorerCommandContext {
    uri?: Uri;
}

interface ScmCommandContext {
    // ??? - Haven't dug into the specifics of this one yet
}

callback: (source: CommandSource, context: {} | EditorCommandContext | ExplorerCommandContext | ScmCommandContext, args: any[]) => any)

With something like the last option, it is very extensible (its easy to add another property to the context without breaking any existing code) and its also easier to document and consume. It would also have the added benefit of providing an extension with insight to in what context a command was executed.

Of course changing the existing callbacks would be a major breaking change, but this could be a new command registration api (with the maybe eventual deprecation of the existing command registration?).

To give a concrete example of the issues outlined above -- I'm currently working on refactoring the commands in GitLens, so that I can add context menu support to SCM items. But this has been proving more complex than I had first anticipated because of the argument structure of the SCM menus, as well as not being able to tell one context from another. So I've been toying with a few different solutions -- from attempting argument inference in a command base class, that would reorganize the arguments into a more structured layout -- to multiple commands registered for different contexts, and then boiling them together in a sort of "meta" command that would provide something like the callback outlined above.

In summary, I feel like the current command callback structure is only going to get harder to deal with as the product evolves and more contexts are added -- so I'm very curious for feedback, alternative solutions, etc.

Thanks!

//cc @jrieken

Metadata

Metadata

Assignees

Labels

*out-of-scopePosted issue is not in scope of VS Codeapiunder-discussionIssue is under discussion for relevance, priority, approach

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions