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

fix(foreach): Remove shell invocation from foreach #136

Merged
merged 9 commits into from
Jun 17, 2024

Commits on Jun 4, 2024

  1. 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
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    4e10117 View commit details
    Browse the repository at this point in the history
  2. Format foreach shell arguments nicely

    Shell escape the arguments so you can tell when arguments have spaces in
    them.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    9527f89 View commit details
    Browse the repository at this point in the history
  3. 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.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    965abb7 View commit details
    Browse the repository at this point in the history
  4. Reword foreach usage for accuracy

    Make it clear that the double-hyphen separator `--` is mandatory and not
    just a good idea.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    c66b5ff View commit details
    Browse the repository at this point in the history
  5. Wrap long usage message

    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.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    25f9a69 View commit details
    Browse the repository at this point in the history
  6. 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.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    52aecd5 View commit details
    Browse the repository at this point in the history
  7. Improve logged command format and add tests

    Wrap braces around the logged command to delimit it clearly. Add unit
    tests to cover argument formatting.
    annettejanewilson committed Jun 4, 2024
    Configuration menu
    Copy the full SHA
    c713c89 View commit details
    Browse the repository at this point in the history

Commits on Jun 11, 2024

  1. Configuration menu
    Copy the full SHA
    4415dd9 View commit details
    Browse the repository at this point in the history

Commits on Jun 17, 2024

  1. Configuration menu
    Copy the full SHA
    80e4b5d View commit details
    Browse the repository at this point in the history