-
Notifications
You must be signed in to change notification settings - Fork 34
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
fix(foreach): Remove shell invocation from foreach #136
Commits on Jun 4, 2024
-
Remove shell invocation from foreach
As discussed in [issue 135], turbolift applies shell escaping inconsistently on the arguments in foreach. Upon consideration, I think there is no good reason to apply shell escaping at all. It makes turbolift more fragile, more confusing and ultimately less flexible. Instead, do not use a shell at all and all these problems disappear. None of the examples in the README use this functionality. If users do need to invoke a shell, for example to do redirection, it is much more explicit and understandable for them if they do it themselves. This PR: * Removes the use of a shell, instead directly executing a subprocess based on the arguments provided. * Removes custom handling of arguments. Users are directed to the standard GNU-style `--` to indicate that everything else on the command-line is an argument, not an option. This makes the code substantially simpler. [issue 135]: Skyscanner#135
Configuration menu - View commit details
-
Copy full SHA for 4e10117 - Browse repository at this point
Copy the full SHA 4e10117View commit details -
Format foreach shell arguments nicely
Shell escape the arguments so you can tell when arguments have spaces in them.
Configuration menu - View commit details
-
Copy full SHA for 9527f89 - Browse repository at this point
Copy the full SHA 9527f89View commit details -
Require -- before foreach command
Since we are deliberately removing custom handling of arguments, users will find that if they don't include a double hypen (`--`) separator before their foreach command, any options they pass to their command will be interpreted by turbolift and either result in errors, or worse, being quietly omitted from their command (e.g. `-v`), which could be surprising and frustrating! To head this off, refuse immediately if `--` either doesn't appear on the command-line, or appears after one or more arguments.
Configuration menu - View commit details
-
Copy full SHA for 965abb7 - Browse repository at this point
Copy the full SHA 965abb7View commit details -
Reword foreach usage for accuracy
Make it clear that the double-hyphen separator `--` is mandatory and not just a good idea.
Configuration menu - View commit details
-
Copy full SHA for c66b5ff - Browse repository at this point
Copy the full SHA c66b5ffView commit details -
Turns out Cobra doesn't wrap usage messages, so they look pretty ugly if we don't pick a pretty conservative width and wrap by hand.
Configuration menu - View commit details
-
Copy full SHA for 25f9a69 - Browse repository at this point
Copy the full SHA 25f9a69View commit details -
Fix [flags] appearing at end of usage
Turns out Cobra searches your usage line for the specific text "[flags]" and if it doesn't find it, shoves it on the end. Gross. For that reason, let's remove explicit callout of any flags and move it back to *before* the double-hyphen.
Configuration menu - View commit details
-
Copy full SHA for 52aecd5 - Browse repository at this point
Copy the full SHA 52aecd5View commit details -
Improve logged command format and add tests
Wrap braces around the logged command to delimit it clearly. Add unit tests to cover argument formatting.
Configuration menu - View commit details
-
Copy full SHA for c713c89 - Browse repository at this point
Copy the full SHA c713c89View commit details
Commits on Jun 11, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 4415dd9 - Browse repository at this point
Copy the full SHA 4415dd9View commit details
Commits on Jun 17, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 80e4b5d - Browse repository at this point
Copy the full SHA 80e4b5dView commit details