joh / when-changed

Execute a command when a file is changed
Other
1.21k stars 107 forks source link

Use get_opt instead of manual arguments parsing #50

Closed leogermond closed 5 years ago

leogermond commented 7 years ago

The argument parsing method of when-changed is lacking: you cannot freely reorder the "-c" argument and you cannot join options together; for example if you want the command foo to run immediately on the current directory, with verbosity on, recursively, you must call

# when-changed -r -v -1 . foo

with these changes you can now also call

# when-changed -rv1 . foo

These changes also make possible reordering the -c option; eg:

# when-changed -rc foo .

Finally I attached two minor changes as well: added -h and --help to usage and fixed the exit status in case of command misuse (bad argument): it is commonly set to 2 in order to differentiate it from more general errors (it is not per any standard so this can be reverted).

joh commented 7 years ago

Great, but looks like it breaks one use-case:

./when-changed -v file1 file2 -c echo %f changed
When 'file1', 'file2', '%f' or 'changed' changes, run 'echo'

Expected behavior:

./when-changed -v file1 file2 -c echo %f changed
When 'file1' or 'file2' changes, run 'echo %f changed'
leogermond commented 7 years ago

You are right, this was tricky to test: I modified the code to add tests for parsing and could get an implementation working. This mean any argument after -c is ignored, so -c acts as a '--' for arguments parsing.

leogermond commented 7 years ago

3rd commit is the most consistent: it performs a pre-parsing to split the options at the right place (otherwise get_opt will try to parse the command arguments, see test_command_c_with_options_2()). I think it makes the code clearer and I fixed some bugs in my implementation this way too. With previous code, adding an option with argument would have failed on some corner cases. I'm thinking a bit ahead of the curve with that one as I'm preparing the code for adding exclude filters.

Let me know what you think :)