-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
changed quoting to double quote outside, escaped double quote inside … #2657
Conversation
All of this manual escaping feels scary. Won't |
In this case, we're dealing with 3 different quoting/escaping rules on windows, all conflicting and fighting with each other. I can't imagine shell-quote-argument making sense on any of that. I explained the rationale for this pr in #2656 |
What worries me is that I think the change you made might break the command on linux and Mac. At least, it reads like you're changing a |
I tested this on ubuntu, i don't have a mac though |
The behaviour should be the same, as the shell's what matters not the OS. It'd be nice if a few people tested the change before we merge this PR, though. Right now I'm traveling and I'm short on time for this. |
This is definitely the wrong way to do escaping in sh and bash. Things within double quotes get evaluated for variable substitution. Single quotes is the right way to go, and it should change
As opposed to with double quotes where this happens:
|
It sounds like platform-specific quoting is needed, where you could just call |
See also #2685 I think it's pretty elegant way to solve this quoting mess. |
That's actually how i did it with my binary wrapper, it works |
Guess we can close this one. |
PR Supporting #2656
Changed quoting for command line invocation of clojure.
Tested working with windows 10 and ubuntu.