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

correct shellQuote on windows #6274

Open
victor-shepardson opened this issue Apr 26, 2024 · 2 comments
Open

correct shellQuote on windows #6274

victor-shepardson opened this issue Apr 26, 2024 · 2 comments
Labels

Comments

@victor-shepardson
Copy link

Motivation

Not exactly a bug since the helpfile is clear, but String.shellQuote is not useful on windows, since single quotes don't work to escape space in cmd.exe. Would it make sense for shellQuote to be platform sensitive like +/+ is?

Description of Proposed Feature

Make shellQuote use double quotes when the platform is windows.

I think it's sufficient to add the quotes, existing double quotes need not be escaped since they are forbidden from paths on windows. Maybe someone more familiar with cmd.exe can weigh in. Should the presence of double quotes in the string cause a warning or error?

Plan for Implementation

shellQuote {
    (thisProcess.platform.name==\windows).if{
        ^"\""++this++"\""
    }{
        ^"'"++this.replace("'","'\\''")++"'"
    }
}

This would be a breaking change on windows.

@victor-shepardson
Copy link
Author

victor-shepardson commented Apr 26, 2024

there is at least one problem with the above, it causes paths with a trailing \ to result in an escaped ", like C:\some\path\ quotes to 'C:\some\path\" but by cmd.exe as C:\some\path\" because of the \"

@victor-shepardson
Copy link
Author

could be like this, with bashShellQuote preserving the old behavior:

cmdShellQuote {
    var str = this;
    // drop trailing slashes so they don't escape the " character
    while{str.endsWith("\\")}{str = str.drop(-1)};
    ^ str.quote
}
bashShellQuote {
    ^ "'"++this.replace("'","'\\''")++"'"
}
shellQuote {
    ^ (thisProcess.platform.name==\windows).if{ this.cmdShellQuote }{ this.bashShellQuote };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants