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

Terminal doesn't handle PowerShell's space escape character correctly #33584

Closed
dbaeumer opened this issue Aug 31, 2017 · 9 comments
Closed

Terminal doesn't handle PowerShell's space escape character correctly #33584

dbaeumer opened this issue Aug 31, 2017 · 9 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities terminal Integrated terminal issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) windows VS Code on Windows issues
Milestone

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Aug 31, 2017

PowerShell supports back tick (`) as a space escape character. Using this when creating a terminal command for PowerShell makes the terminal fail.

For example dir folder with spaces

The command that is constructed starting the terminal is:

capture

@vscodebot vscodebot bot added the terminal Integrated terminal issues label Aug 31, 2017
@Tyriar
Copy link
Member

Tyriar commented Aug 31, 2017

Not sure we should support this on the task/terminal config level as it seems very specific to PowerShell. What do you think @daviwil?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Aug 31, 2017
@dbaeumer
Copy link
Member Author

I am not sure but from some testing I did in the past I had the impression that the space escaping for bash which uses \ is supported. The only difference is that PowerShell uses a different escape character.

@daviwil
Copy link
Contributor

daviwil commented Aug 31, 2017

Hey guys, how does this come up in practice? You'd definitely need the backtick character to escape spaces when invoking PowerShell commands but typically people would surround strings containing spaces with single- or double-quotes, removing the need for space escaping. Any way you could use quotation marks instead of escaping spaces?

@Tyriar
Copy link
Member

Tyriar commented Aug 31, 2017

Nothing special is done to support \ escape on Linux/macOS as I think it's supported at a native level. I think the backtick escaping is PowerShell specific which is why it doesn't work, it would be a little weird imo if the shell was cmd.exe and backtick was being escaped in the args. I believe single and double quote escaping works just fine in the terminal args.

@dbaeumer
Copy link
Member Author

dbaeumer commented Sep 1, 2017

The backtick character is supported on native level in PowerShell as well. I think something is simply lost when the command line is finally passed to the PowerShell executable making the command fail. May be I expressed myself incorrectly. I am not asking to support backtick as a general space escaping mechanism and that we do some smartness to make backtick work on bash or cmd. But I do think that we should pass a string containing a backtick correctly to the PowerShell executable so that it can do the right thing.

It came up for two reason: it was used in a task executed in a PowerShell environment. As a work around I told the user to use single quotes, but it makes it more complicated if the command uses already quotes. And I wanted to use it as well. When we resolve variables and they contain spaces the easiest on bash and PowerShell is to escape the space (e.g. consider ${file} resolves to a path containing spaces)). So for example a command like 'xxx ${file}' (notice the quotes) could then be 'xxx file` with` space'. Putting quotes around the file name is more complicated.

@vscodebot vscodebot bot closed this as completed Sep 8, 2017
@vscodebot
Copy link

vscodebot bot commented Sep 8, 2017

This issue has been closed automatically because it needs more information and has not had recent activity. Please refer to our guidelines for filing issues. Thank you for your contributions.

@dbaeumer dbaeumer removed the info-needed Issue requires more information from poster label Sep 8, 2017
@dbaeumer
Copy link
Member Author

dbaeumer commented Sep 8, 2017

Removing needs more info and reopening.

@dbaeumer dbaeumer reopened this Sep 8, 2017
@Tyriar
Copy link
Member

Tyriar commented Sep 8, 2017

I guess this would involve tinkering with the format to see what PowerShell expects then. Seems related to microsoft/node-pty#47, maybe backticks need to be escaped like https://github.com/Tyriar/node-pty/pull/47/files#diff-103714ba8ecf05b4f0425c2204698064R24?

@Tyriar Tyriar added this to the October 2017 milestone Sep 8, 2017
@Tyriar Tyriar added the windows VS Code on Windows issues label Sep 8, 2017
@Tyriar Tyriar modified the milestones: October 2017, Backlog Oct 30, 2017
@Tyriar Tyriar added feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Dec 26, 2017
@alexr00
Copy link
Member

alexr00 commented Sep 12, 2018

We would need to add special handling for PowerShell's backtick escaping since using backtick to escape space isn't universal. You can easily work around this by passing in a string instead of string[] which was added in #57971

@alexr00 alexr00 closed this as completed Sep 12, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 27, 2018
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 help wanted Issues identified as good community contribution opportunities terminal Integrated terminal issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

4 participants