Use argument processor for the commands called from plugins #6631
Conversation
Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
vzhukovs
added
enhancement
vzhukovs
requested review from
azatsarynnyy,
vparfonov,
akosyakov and
AndrienkoAleksandr
Please elaborate how we can check in with local Theia. Also have to be sure that it does not cause any regressions. |
|
I don't think it is right place, there is nothing like that in VS Code implementation and it still works: https://github.com/microsoft/vscode/blob/ba40bd16433d5a817bfae15f3b4350e18f144af4/src/vs/workbench/api/common/extHostCommands.ts#L113 We are overlooking something. |
| @@ -100,6 +100,7 @@ export class CommandRegistryImpl implements CommandRegistryExt { | |||
|
|
|||
| // tslint:disable:no-any | |||
| executeCommand<T>(id: string, ...args: any[]): PromiseLike<T | undefined> { | |||
| args = args.map(arg => this.argumentProcessors.reduce((r, p) => p.processArgument(r), arg)); | |||
There was a problem hiding this comment.
We already do it in this.executeLocalCommand, so we will run it twice.
I don't understand why we will run it for main side commands?
An idea behind argument processors is to avoid sending data which is already in the plugin host for local commands not to transform something for the main side. You can do it by overriding services in the main side already, like CommandRegisry.execute.
There was a problem hiding this comment.
Yeah, you're right, overriding service might be better solution in this case. I've updated code in following PR on che-theia side: https://github.com/eclipse/che-theia/pull/562/files.
The main problem I'm trying to solve is that, in our infrastructure each plugin runs in separate container. For example, there is the kubernetes plugin, which provides command to see the diff for the selected resource (opened in the editor). Right part for the diff takes directly from the opened editor. But left part - there is a resource stored to temporary file (in /tmp directory) in container, where plugin hosts. So we're having following situation. Kubernetes plugin calls vscode.diff command with two arguments: 1) path to the file, opened in the editor, everything is fine; 2) path to the file which doesn't exists in theia container, but stored in different container in /tmp directory. So we need to modify the file scheme for second argument. And with custom file content provider open this file in the diff viewer. Smth like that.
But I've tried to override the call of executeCommand method and that works for the che-theia use case. So this PR can be safely closed.
There was a problem hiding this comment.
@vzhukovskii ok, glad that you find a way to resolve the issue!
What it does
Use process argument before command executes. This changes proposal allows command registry use mechanism of argument processor.
This PR fixes eclipse-che/che#13044
Signed-off-by: Vladyslav Zhukovskyi vzhukovs@redhat.com
How to test
Register custom argument processor and call any command from plug-in.
Review checklist
Reminder for reviewers