-
Notifications
You must be signed in to change notification settings - Fork 34.7k
Description
- 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