Vscode: terminal.integrated.shellArgs.windows user setting ignores quotes, and escape sequences

[AArnott]

Issue Type: Bug

As described in this blog post, add these settings to the User Settings json file (adjusting for your actual VS 2017 install dir as necessary):

    "terminal.integrated.shell.windows": "cmd.exe",
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\release\\Common7\\Tools\\VsDevCmd.bat"
    ],

As (somewhat) expected, this gets the VS Code Terminal running with the VsDevCmd.bat script started first. But this doesn't include quotes around the VsDevCmd.bat script. While this works in the simple Terminal window case, it causes any VS Code "tasks" to fail with this bizarre error:

> Executing task: d:\git\Nerdbank.Streams\src\nerdbank-streams\node_modules\.bin\tsc.cmd -p d:\git\Nerdbank.Streams\src\nerdbank-streams\tsconfig.json <

'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

So we need to add quotes, apparently. But here's where the VS Code bug comes in. Given this is JSON, I should be able to add quotes by escaping them:

    "terminal.integrated.shellArgs.windows": [
        "/k",
        "\"C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\release\\Common7\\Tools\\VsDevCmd.bat\""
    ],

But VS Code doesn't interpret the escape sequences and instead keeps the backslashes in the string, producing this error when I open a terminal window:

'\"C:\Program Files (x86)\Microsoft Visual Studio\2017\release\Common7\Tools\VsDevCmd.bat\"' is not recognized as an internal or external command,
operable program or batch file.

Very oddly, this syntax is accepted:

    "terminal.integrated.shellArgs.windows": [
        "/k "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\release\\Common7\\Tools\\VsDevCmd.bat\""
    ],

Note the above isn't even valid JSON since the inside quotes aren't properly escaped. Yet VS Code happily runs with this and spawns the process... with the inner quotes dropped, which means it doesn't actually solve my problem because once again, the VS Code tasks fail because of the missing quotes around that script.

Can we please get this JSON file properly interpreted, including escape sequences, so that we can use quotes in these shell args?

This seems closely related to #44152 but as that is focused on tasks inheriting a common environment, I think a separate issue tracking just supporting using quotes inside shell arguments is worthwhile and would solve my issue.

VS Code version: Code 1.26.1 (493869ee8e8a846b0855873886fc79d480d342de, 2018-08-16T18:38:57.434Z)
OS version: Windows_NT x64 10.0.17134

System Info

|Item|Value|
|---|---|
|CPUs|Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz (8 x 2112)|
|GPU Status|2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
rasterization: enabled
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled|
|Memory (System)|15.93GB (2.75GB free)|
|Process Argv|C:\Users\andarno\AppData\Local\Programs\Microsoft VS Code\Code.exe ..|
|Screen Reader|no|
|VM|0%|

Extensions (29)

Extension|Author (truncated)|Version
---|---|---
npm-intellisense|chr|1.3.0
path-intellisense|chr|1.4.2
vscode-markdownlint|Dav|0.20.0
jshint|dba|0.10.19
vscode-eslint|dba|1.5.0
githistory|don|0.4.2
xml|Dot|2.3.2
gitlens|eam|8.5.6
remotehub|eam|0.2.0
EditorConfig|Edi|0.12.4
tslint|eg2|1.0.38
vscode-npm-script|eg2|0.3.5
git-project-manager|fel|1.6.1
docker-linter|hen|0.5.0
beautify|Hoo|1.4.2
docomment|k--|0.1.2
azure-account|ms-|0.4.3
cpptools|ms-|0.18.1
csharp|ms-|1.15.2
liveshare-logviewer|ms-|0.2.1
PowerShell|ms-|1.8.3
team|ms-|1.136.0
debugger-for-chrome|msj|4.8.2
vscode-docker|Pet|0.1.0
vscode-versionlens|pfl|0.21.1
java|red|0.29.0
vscode-icons|rob|7.25.0
gitconfig|sid|2.0.0
vsc-docker|Zim|0.34.0

(1 theme extensions excluded)

[Tyriar]

This is related to the way that we're computing the "Command Line" string in node-pty, here's the code pointer:

https://github.com/Microsoft/node-pty/blob/cafed13d456fa37df46a9dc26f9a5ebdfe88f3f6/src/windowsPtyAgent.ts#L113-L161

The fix here may be to allow passing in a raw string like is allowed in tasks https://github.com/Microsoft/node-pty/blob/cafed13d456fa37df46a9dc26f9a5ebdfe88f3f6/src/windowsPtyAgent.ts#L121

Interested in your thoughts on this @dbaeumer; should terminal.integrated.shellArgs.windows be a string[] | string?

[dbaeumer]

Since this is already OS & Shell specific I would stick with one string which is exactly pass this way to the shell. Then it is clear how it is for example quoted. I see no benefit for a []

[Tyriar]

@dbaeumer you mean allow a string only for Windows? This would break people if we didn't support string[] anymore.

[AArnott]

Changing the way you're interpreting the string(s) might also break people (since quotes that you're dropping would suddenly not be dropped and cause a JSON parsing failure instead). But I think it's a good change regardless. You might have a helpful error message that links to a doc describing the change though.

[dbaeumer]

@Tyriar my misunderstanding. Then it should be string[] | string.

[Tyriar]

@sandy081 is it possible to have VS Code settings be union types. I want to have a string[] | string in a single setting but still have the type checking/warnings when it's wrong.

[sandy081]

This has to be supported by JSON Schema.. @aeschli ?

[aeschli]

Try something like:

{
    "properties": {
        "terminal.integrated.shellArgs.windows": {
            "anyOf": [
                {
                    "type": "array",
                    "items": {
                        "type": "string",
                        "description": "%arg.desc%"
                    },
                },
                {
                    "type": "string",
                    "description": "%arg.desc%"
                }
            ],
            "default": []
        }
    }
}

[alexr00]

I tried this out, and it looks like even using string[] | string doesn't solve this. The args string is passed into winpty spawn as expected with the escaped characters converted. The bug in winpty needs to be addressed to solve this.

[AArnott]

If you're using child_process.spawn (or similar), the removal of the quotes may be by design unless you set windowsVerbatimArguments: true as documented here.

[alexr00]

Looks like I was hitting a separate issue. string[] | string does work around this.

[alexr00]

With the work around in #57971 you can put all the arguments in one string instead of an array. With Microsoft/node-pty#222 , the actual escaping issue is fixed.

[chrmarti]

@dbaeumer The task does not run, maybe that's expected because the task is passed to the shell as a parameter?

My settings:

    "terminal.integrated.shell.windows": "cmd.exe",
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "\"C:\\devel\\fresh\\sp ace\\echo.bat\""
    ]

My tasks.json:

{
    "version": "2.0.0",
    "tasks": [
        {
            "taskName": "echosomething",
            "type": "shell",
            "command": "echo",
            "args": [
                "HERE"
            ]
        }
    ]
}

echo.bat:

echo TEST!

The output is:

> Executing task: echo HERE <


C:\devel\fresh>echo TEST!
TEST!

C:\devel\fresh>

When I remove the shell arguments, the task runs as expected.

[chrmarti]

Also: With the quotes the terminal (without task) now opens with an error: '\"C:\devel\fresh\sp ace\echo.bat\"' is not recognized as an internal or external command, operable program or batch file.

That seems unexpected. Reopening.

[Tyriar]

@chrmarti this is not meant to work, the quoting should happen for you:

    "terminal.integrated.shell.windows": "cmd.exe",
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "\"C:\\devel\\fresh\\sp ace\\echo.bat\""
    ]

Verification steps:

    "terminal.integrated.shell.windows": "cmd.exe",
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "C:\\devel\\fresh\\sp ace\\echo.bat"
    ]

[chrmarti]

Without the quotes I see the originally reporter error when running a task:

> Executing task: echo HERE <

'C:\devel\fresh\sp' is not recognized as an internal or external command,
operable program or batch file.

C:\devel\fresh>

[alexr00]

There is an issue with terminalTaskSystem which is not doing any escaping. However I think there's also some contention between "terminal.integrated.shellArgs.windows" and tasks. @chrmarti, your expectation is that your terminal starts with your specified args and then runs your task, correct?

As far as I understand, the way it actually works is the task command + parameters are passed as additional parameters when starting your terminal with shell args. So, the whole line would look something like(if we were escaping it correctly):

cmd.exe /k "c:\users\alros\documents\sp ace\echo.bat" "echo HERE"

I assume that instead you wanted to run the shell args and then the task sequentially. It seems like for tasks we should start the terminal with the shellArgs then execute the task instead of putting them all in one command line. @dbaeumer and @Tyriar, what do you think?

[Tyriar]

It seems like for tasks we should start the terminal with the shellArgs then execute the task instead of putting them all in one command line

@alexr00 tasks need to finish though and they won't if you just launch the shell and run some stuff. Also I think that would be a pretty radical departure from the current design.

[alexr00]

In that case, the task and shellArgs that @chrmarti tested with shouldn't work.

[alexr00]

The original issue where the terminal was incorrectly escaping has been fixed. Opened a new issue to figure out how to reconcile shellArgs and tasks. #59789

[roblourens]

What is the correct format of that setting now? I tried both (with escaped double quotes and without) and neither seems to work. I either get 'C:\Program' is not recognized as an internal or external command'C:\Program' is not recognized as an internal or external command or with escaped quotes, opening a normal terminal complains that '\"C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\VsDevCmd.bat\"' is not recognized as an internal or external command, operable program or batch file.

[alexr00]

Looks like this has completely regressed. It only ever worked in insiders, so moving this to November.

[joaomoreno]

After discussing this with @alexr00, it is my understanding that this has been an issue in stable for long. Given that we're at the end of endgame and it's not a regression nor severe, I'll push this out to November.

[alexr00]

@roblourens, can you confirm whether C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\VsDevCmd.bat actually exists? I realized I don't have that file and tried again with a file I do have and it works.

[roblourens]

Yeah it does exist

[damianog]

EUREKA!

I managed to launch the dev cmd by adding an env variable

"terminal.integrated.env.windows": {
        "VsDevCmd": "\"C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Community\\Common7\\Tools\\VsDevCmd.bat\""
    },
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "%VsDevCmd%"
]

[AArnott]

Very nice, @damianog. I made a couple changes so I could use the VsDevCmd script and still launch powershell:

    "terminal.integrated.env.windows": {
        "VsDevCmd": "\"C:\\Program Files (x86)\\Microsoft Visual Studio\\IntPreview\\Common7\\Tools\\VsDevCmd.bat\""
    },
    "terminal.integrated.shell.windows": "C:\\WINDOWS\\system32\\cmd.exe",
    "terminal.integrated.shellArgs.windows": [
        "/k",
        "%VsDevCmd%",
        "&&",
        "%windir%\\system32\\windowspowershell\\v1.0\\powershell.exe"
    ],

[alexr00]

I still can't repro this. I'm using this:

"terminal.integrated.shell.windows": "c:\\Windows\\System32\\cmd.exe",
"terminal.integrated.shellArgs.windows": [
    "/k",
    "C:\\Program Files (x86)\\Microsoft\\path with more spaces\\test.bat"
],

Can one of the folks who is still seeing space escape issues try this out?

[AArnott]

@alex00 Are you trying my original repro steps? Remember that this alone will allow the terminal to launch, but it's the VS Code "tasks" to fail.