[Modern Commanding] Custom CommandArgs Factories

[olegtk]

In modern editor commanding CommandArgs serves as both unique command identifier and a container for command specific arguments. While most commands do not accept any input arguments and their CommandArgs don't need any command specific arguments beside text view and subject buffer, some commands do. A prime example is the TypeCharCommandArgs, which contains the typed char. Extracting that typed character and instantiating such CommandArgs is host specific (in VS it's passed via IOleCommandTarget.Exec’s pvaIn argument) and currently handled by the editor host. At the moment therefore there is no way for extenders to create a new command that accepts host specific input.

Proposed solution

Keep command handlers cross IDE and introduce the concept of host specific CommandArgs factories. Such a factory allows creating CommandArgs instances using custom logic that utilizes IDE specific command input such as typed char in TypeCharCommandArgs or command arguments when a command is called via Command Window in VS. In VS such a factory gets access to IOleCommandTarget.Exec’s nCmdexecopt and pvaIn arguments. Other hosts will need to provide a similar functionality.

Here is a sample CommandArgs facrtory:

    [Export(typeof(IVsCommandArgsFactory))]
    [Command(typeof(ShowContextMenuCommandArgs))]
    internal class ShowContextMenuCommandArgsFactory : IVsEditorCommandArgsFactory<ShowContextMenuCommandArgs>
    {
        /// <summary>
        /// This one is used in <see cref="ICommandHandler{T}.GetCommandState(T)"/>.
        /// </summary>
        public ShowContextMenuCommandArgs CreateCommandArgs(ITextView textView, ITextBuffer subjectBuffer)
        {
            return new ShowContextMenuCommandArgs(textView, subjectBuffer);
        }

        /// <summary>
        /// This one is used in <see cref="ICommandHandler{T}.ExecuteCommand(T, CommandExecutionContext)"/>.
        /// </summary>
        public ShowContextMenuCommandArgs CreateCommandArgs(ITextView textView, ITextBuffer subjectBuffer,
            uint nCmdexecopt, IntPtr pvaIn, IntPtr pvaOut)
        {
            if (pvaIn != IntPtr.Zero)
            {
                //the coordiantes are passed as variants containing short values. The y coordinate is an offset sizeof(variant)
                //from pvaIn (which is 16 bytes)
                object xCoordinateVariant = Marshal.GetObjectForNativeVariant(pvaIn);
                object yCoordinateVariant = Marshal.GetObjectForNativeVariant(pvaIn + 16);
                short? xCoordinate = xCoordinateVariant as short?;
                short? yCoordinate = yCoordinateVariant as short?;
                if (xCoordinate.HasValue && yCoordinate.HasValue)
                {
                    return new ShowContextMenuCommandArgs(textView, subjectBuffer, xCoordinate.Value, yCoordinate.Value);
                }
            }

            return new ShowContextMenuCommandArgs(textView, subjectBuffer);
        }
    }

Note that it creates 2 instances of ShowContextMenuCommandArgs, one that doesn't contain any input and is intended to be passed to ICommandHandler<T>.GetCommandState(T) methods and another, which does contain the input and is intended to be passed to ICommandHandler<T>.ExecuteCommand(T, CommandExecutionContext). This weird dichotomy is required because in VS IOleCommandTarget.QueryStatus doesn't have access to command arguments. Alternatively we could drop first method entirely and require such CommandArgs classes to have a constructor that only accepts ITextView and ITextBuffer, but I prefer to keep all factory logic in one place.

Alternative solution would be to stop treating CommandArgs as both command ID and command input input container and leave it being just command ID and add CommandInput concept. Then ICommandHandler.GetCommandState will only get CommandArgs, but ICommandHandler.ExecuteComamnd - both CommandArgs and CommandInput. That goes against current commanding design in a pretty fundamental way though.

Additionally note [Command] attribute on the factory. Unlike command handlers it doesn't make sense for CommandArgs factories to have [ContentType]/[TextView] attributes so we need some metadata to avoid loading all factories just to determine which can produce a specific CommandArgs. Alternatively we can introduce a custom attribute for exporting CommandArgs factories:

[ExportCommandArgsFactory(typeof(ShowContextMenuCommandArgs))]
internal class ShowContextMenuCommandArgsFactory : IVsEditorCommandArgsFactory<ShowContextMenuCommandArgs>
{
...
}

[KirillOsenkov]

Can you remind us why we need the args factories? It seems like we should be able to instantiate commands just by calling the constructor?

[olegtk]

So for example an extender wants to add DiffFiles command, which accepts 2 file paths as an input. How input is provided in host specific (in VS that input can be passed in Command Window, in VS for Mac in a some different way), but we want command handler to be as cross IDE as possible so all host specific input should be processed in some host specific layer and encapsulated in DiffFilesCommandArgs. The factory here is an extensibility point to address that.