Conversation
|
@mcblum @pcrockett @meleu |
|
This looks great to me, thank you for getting after it so quickly! |
|
Will this solve your use case? |
|
hm, i dunno. i think the VAST majority of scripts out there have no need to manipulate raw args. so adding a logic branch just for this one edge case seems a bit wonky to me. i tend to lean more toward something like this: raw_args="$@"
initialize # has ability to manipulate raw_args if desired
run "${raw_args[@]}"this wouldn't clutter up my scripts so much, but would give "edge-case" folks the flexibility they want. wdyt? |
Yes, thank you. Totally agree. |
|
Done. I called the variable
|
|
Ready to merge? |
pcrockett
left a comment
There was a problem hiding this comment.
Looks good to me. Small nit, not blocking a merge:
I'm not an enormous fan of the command_line name, as that implies to me that it includes the full command (including the script name / path). Something with "args" in the name (i.e. command_line_args) makes more sense to me. It's a matter of preference / taste though.
And other by-the-way comments:
- I'm not sure how you like to handle PRs, but you might consider updating the original description with the current state of the PR, so future people don't need to read all the way through to know what actually got merged.
- I actually unsubscribed from this PR because I was getting too many notifications (every time you pushed a commit -- not your fault, just GitHub's). I luckily checked it again today and saw you asked me for another look. In the future, (re-)requesting a review from me would be more likely to get my attention.
cc #643
This PR allowed overriding the raw input args before they are fed into
run.It mainly changes the script startup to this: