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

For calls to vscode.tasks.executeTask that use shell tasks, treat command as we treat the arguments in terms of the use of ShellQuotedString. #10997

Merged
merged 4 commits into from
Apr 22, 2022

Conversation

jdmchp
Copy link
Contributor

@jdmchp jdmchp commented Apr 6, 2022

What it does

This pull request fixes #10805

It basically removes the limitation of a command having to be a string.
It does this by treating the command the same way as the arguments are currently treated: for ShellQuotedStrings keep the value of the command, and then later on in the backend, quote strongly.

The reason for this fix is to allow the ms-vscode.makefile-tools extension to work with Theia. This extension runs make and uses a ShellQuotedString as a command.

How to test

Try to use the ms-vscode.makefile-tools extension, it will not work. Apply the patch, it will start working.

Before the patch, any call to vscode.tasks.executeTask where a command is passed as a ShellQuotedString would fail. For example:

let myTaskOptions: vscode.ShellExecutionOptions = {env: process.env as { [key: string]: string }, cwd: "/home/jose/tmp/foo"};
const quotingStlye: vscode.ShellQuoting = vscode.ShellQuoting.Strong;
let myTaskCommand: vscode.ShellQuotedString = {value: "make with spaces", quoting: quotingStlye};
let shellExec: vscode.ShellExecution = new vscode.ShellExecution("make with spaces", myTaskOptions);
let myTask: vscode.Task = new vscode.Task({type: "shell", group: "build", label: "my label"},
vscode.TaskScope.Workspace, "makefileBuildTaskName", "makefile", shellExec);

myTask.problemMatchers = [];
myTask.presentationOptions.showReuseMessage = true;

await vscode.tasks.executeTask(myTask);

would fail with:

throw new Error('Converting ShellQuotedString command is not implemented');

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the review guidelines
    Commands with spaces, for example, now can be passed to executeTask and they will work OK in windows or *nix.

…alue to the backend. And in the backend, quote strongly. Signed-off-by: Jose Diaz <[email protected]>

Added test to convert shell task with quoted command
@jdmchp jdmchp changed the title Treat command as we treat args when running a shell task: pass the .v… For calls to vscode.tasks.executeTask that use shell tasks, treat command as we treat the arguments in terms of the use of ShellQuotedString. Apr 6, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdmchp thank you for your first contribution to the project!

Please be sure to address the test failures introduced with the changes and we will review shortly afterwards :)

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes work well for me, I confirmed that it works when using my custom extension
(shell-quote-0.0.1.zip). I only have minor commands about the code.

packages/plugin-ext/src/plugin/type-converters.spec.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/type-converters.spec.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/type-converters.spec.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/type-converters.ts Outdated Show resolved Hide resolved
@vince-fugnitto vince-fugnitto added tasks issues related to the task system vscode issues related to VSCode compatibility labels Apr 12, 2022
@jdmchp
Copy link
Contributor Author

jdmchp commented Apr 22, 2022

Hello @vince-fugnitto , I pushed changes using the feedback you gave me.
Please let me know if more work is needed. Thanks.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdmchp thank you for your patience and first contribution to the project! :)

@vince-fugnitto vince-fugnitto merged commit 7adb00c into eclipse-theia:master Apr 22, 2022
@vince-fugnitto vince-fugnitto added this to the 1.25.0 milestone Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tasks issues related to the task system vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to pass a QuotedString as a command to tasks.executeTask
3 participants