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

Support onDidExecuteCommand #1431

Closed
waderyan opened this issue Dec 17, 2015 · 34 comments
Closed

Support onDidExecuteCommand #1431

waderyan opened this issue Dec 17, 2015 · 34 comments
Assignees
Labels
feature-request Request for new features or functionality release-notes Release notes issues

Comments

@waderyan
Copy link

I am building an extension for VS Code. My extension will show the key bindings associated to a recently action (I am a fan of this for helping me learn key bindings).

Looking through the documentation there seems to be a way to access the commands in the editor, but no way that I can find to listen for actions / commands as they are invoked.

I would like the ability to listen for all user actions and then react to their actions.

@waderyan waderyan added the feature-request Request for new features or functionality label Dec 17, 2015
@egamma
Copy link
Member

egamma commented Dec 17, 2015

This is currently supported by calling this method:

vscode.commands.getCommands(filterInternal?: boolean): Thenable<string[]>;

Does this unblock you?

@waderyan
Copy link
Author

Not quite. From what I can tell in the docs, this function will return a list of commands. What I want is to listen to commands as they happen.

My specific use case is:

  • Listen for commands
  • When command occurs, determine if their is a key binding for that command
  • Display in the status bar (or some UI element) the key binding for the command the user just executed.

@egamma egamma added the api label Dec 18, 2015
@egamma egamma added this to the Backlog milestone Dec 18, 2015
@egamma
Copy link
Member

egamma commented Dec 18, 2015

Got it, this is currently not supported.
@jrieken fyi.

@jrieken jrieken changed the title Add ability in extension api to listen to all commands Support onWillExecuteCommand and onDidExecuteCommand Apr 29, 2016
@siegebell
Copy link

I would also like to see this feature.

I am writing an extension that needs to monitor cursor movements and make slight cursor position adjustments on occasion. The only way I've found to do this is to override the keybinding for e.g. 'left', call 'cursorLeft' from my command, and then validate the cursor's position.

Even without validation, intercepting 'left' and then immediately calling 'cursorLeft' from an extension causes the UI to be much less responsive than binding directly 'left' to 'cursorLeft'. (Because extensions are given low priority.)

@jrieken
Copy link
Member

jrieken commented Jul 14, 2016

@johnfn for the VIM extension do you need onDidExecuteCommand, or onWillExecuteCommand, or both? The first (onDidExecuteCommand) is fairly easy to implement but the latter raises many questions...

@johnfn
Copy link
Contributor

johnfn commented Jul 15, 2016

@jrieken I assume this is for tab complete? If so, post-trigger would be the most useful.

@rebornix
Copy link
Member

rebornix commented Jul 20, 2016

@jrieken we will benefit a lot with onDidExecuteCommand, for example, code snippet then we know how to handle those selections.

// Remove words which are wrong.

@jrieken
Copy link
Member

jrieken commented Jul 21, 2016

@johnfn @rebornix not quite sure what you guys mean by 'Tab complete'?

@rebornix
Copy link
Member

@jrieken A common user case is like:

  1. User types ab in a TypeScript file
  2. Code will suggest abstract
  3. User press <Tab>, Code will do autocomplete and we will get abstract.
  4. User press . to repeat previous action.

Since the only user does is pressing a, b, Tab but we have no idea about the final result of these three keystrokes. so when we repeat previous action, the result will be just another ab, which is not what we want. The reason that we don't get <Tab> is we are listening to type command and <Tab> (Up, Down, Left, Right, etc) will not be passed to us.

So if we can bind handlers to onDidExecuteCommand and we know it's an autocomplete, maybe we can put it in our cached history and repeat correctly?

@alexdima
Copy link
Member

alexdima commented Aug 16, 2016

@rebornix I am not convinced that "spying" on commands will help you in any meaningful way. For example, in the example you make, where a user types ab followed by <Tab>, you will find out the command acceptSelectedSuggestion has executed.

But IMHO this does not help you to repeat the action, as the command acceptSelectedSuggestion is completely contextual. Perhaps the suggest widget has even closed in the meantime, so executing it programmatically will result in a no-op.

Perhaps . can be supported only for vim commands rather than for any command plugged into vscode? Would that be a vim adoption blocker?

@rebornix
Copy link
Member

@alexandrudima Thanks for your input. I agree that acceptSelectedSuggestion execution doesn't solve this problem. But since we are only monitoring type command then we have no idea what's the exact content being inserted into the editor, we can't implement . perfectly.

. is a Vim adoption blocker as well as u (undo). Since you are enriching Code's undo/redo commands, will there be any additional info to help implement repeat?

@alexdima
Copy link
Member

@rebornix My question stands: "Perhaps . can be supported only for vim commands rather than any command plugged into vscode?".

There are a lot of commands in VSCode out of the box and even more from extensions. Types of commands that I can think of:

  • commands that edit the document: e.g. toggle line comment (ctrl+/ or cmd+/)
  • commands that change the editor "view" state: e.g. actions.find (ctrl+f or cmd+f), find all references, show hover, toggle word wrap
  • commands that change the workbench state: e.g. open markdown preview to the side, show references, go to definition, hide sidebar

There are also edits to the document that don't stem from a command (e.g. format on type)

The commands are not categorized statically in any meaningful way. They execute and they do something. Sometimes, that results in a document edit (that can be observed via onDidChangeTextDocument).

Also note the textEditor.edit API that is currently in place. The only information I get from that are the edits to be aplied to a text document, which are immediately reflected in onDidChangeTextDocument.

Is it possible to point me to an example/explanation of how . should work in Vim?

@johnfn
Copy link
Contributor

johnfn commented Aug 18, 2016

The big thing we need is the ability to know that an action such as tab complete or snippet insert or any other sort of text change has taken place. If you guys can't figure out exactly how to let us repeat it, that's okay; we can just manually diff the document and figure out what happened (most of the time).

Quick explanation for .: The idea is that it allows you to repeat an action somewhere else in the document. So if you inserted some text on the first line, then navigated to the last line, . would add the text down there. If you tab-completed a word on the first line and hit . on the last line, it'd insert the full word. etc, etc. . only concerns itself with text-related actions.

@jrieken
Copy link
Member

jrieken commented Aug 18, 2016

only concerns itself with text-related actions

Isn't that an argument for the TextDocumentChangeEvent? We could maybe enrich it with a source-thing - similar to TextEditorSelectionChangeEvent#kind. That would indicate (somehow?) that this textual change was done because of a command and the .-feature would then remember it for repeated execution

@alexdima
Copy link
Member

@jrieken 👍 However, the source is currently a string (not an enum) and at least for edits coming from the extension host the source is set to MainThreadTextEditor -- see https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/api/node/mainThreadEditorsTracker.ts#L342

@jrieken
Copy link
Member

jrieken commented Aug 18, 2016

As long as the strings being sort well defined we can do something like this

@jrieken
Copy link
Member

jrieken commented Aug 22, 2016

@johnfn @rebornix I have extracted #10801 for the last bit of this discussion since this issue is still about events for command execution.

@jrieken
Copy link
Member

jrieken commented Apr 15, 2019

Most (not sure if all) should be named _internal_command_delegation_TIMESTAMP, so in theory ignore _'ed command should work

@hedgerh
Copy link
Contributor

hedgerh commented Apr 15, 2019

Pushed up my work and opened up a RFC/WIP PR of what I came up with. Interesting enough, none of the internal commands actually go through the event emitter in the command service, so they don't surface on the extension side. I verified that internal commands were firing, just not through the emitters.

Interested if there are any performance implications with talking between the two processes like this. Possible enhancement would be to allow a glob/regex/array of strings/wildcard as the first param that would act as a filter for what events surface to the extension.

@jrieken jrieken changed the title Support onWillExecuteCommand and onDidExecuteCommand Support onDidExecuteCommand Apr 30, 2019
@jrieken jrieken removed the *out-of-scope Posted issue is not in scope of VS Code label Apr 30, 2019
@jrieken jrieken reopened this Apr 30, 2019
@jrieken
Copy link
Member

jrieken commented Jul 24, 2019

This PR is merged but there is some little (design) work left

  • revive/massage arguments
  • decide if we should hide private/underscore commands
  • deal with the internal command delegation commands
  • add some more tests
  • understand perf implications of sending arguments around

@jrieken
Copy link
Member

jrieken commented Jul 29, 2019

Closing this for July testing

@jrieken jrieken added the verification-needed Verification of issue is requested label Jul 29, 2019
@alexr00
Copy link
Member

alexr00 commented Jul 31, 2019

This seems to work well, but I'm seeing the event get triggered twice when running this hello world command:

export function activate(context: vscode.ExtensionContext) {
	// Use the console to output diagnostic information (console.log) and errors (console.error)
	// This line of code will only be executed once when your extension is activated
	console.log('Congratulations, your extension "helloworld-sample" is now active!');

	// The command has been defined in the package.json file
	// Now provide the implementation of the command with registerCommand
	// The commandId parameter must match the command field in package.json
	const disposables: vscode.Disposable[] = [];
	disposables.push(vscode.commands.registerCommand('extension.helloWorld', () => {
		// The code you place here will be executed every time your command is executed

		// Display a message box to the user
		vscode.window.showInformationMessage('Hello World!');
	}));

	disposables.push(vscode.commands.onDidExecuteCommand((e: vscode.CommandExecutionEvent) => {
		console.log(e.command);
	}));

	context.subscriptions.push(...disposables);
}

The command isn't actually running twice, but the listener for onDidExecuteCommand does get called twice.

Bug: #78274

@alexr00 alexr00 added the verified Verification succeeded label Jul 31, 2019
@jrieken jrieken added the release-notes Release notes issues label Jul 31, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 12, 2019
@jrieken jrieken removed api api-proposal verified Verification succeeded verification-needed Verification of issue is requested labels Dec 3, 2019
@jrieken jrieken removed this from the July 2019 milestone Dec 3, 2019
@jrieken
Copy link
Member

jrieken commented Dec 3, 2019

We have decided against this API: #78091 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality release-notes Release notes issues
Projects
None yet
Development

No branches or pull requests

10 participants