Drop current call syntax in favor of double dashed options/arguments separator

[samuelgrigolato]

Issue #6 brought cli-args support to dose using click. Consider the following - currently accepted with a deprecation warning - dose call:

dose tox --help

How can we securely tell that everything after tox is supposed to be treated as arguments instead of dose options? Let's change this call a little:

dose --cli tox --help dose --no-cli tox --help

What should happen? "--no-cli" is a switch option, but "--cli" receives a parameter. So, heuristically, we would assume that dose should be called using "tox" cli and display help text in the first case, but run headless the command "tox --help" in the second. One may argue that this doesn't sound quite right.

The proposal is to deprecate this call style (dropping it at the next major release), in favor of forcing the user to double-dash arguments (the test_command) from options:

dose --cli -- tox --help (will cause an error, like we would expect as an user) dose --no-cli -- tox --help

The double-dash allow us to be more precise and less error-prone when calling dose.

Note that this issue is starting as a discussion. There's no assumption that this line of thought is the way to go.

[samuelgrigolato]

Maybe we should consider adding a "--test" option, removing entirely the ability to pass the test command as an argument? This would simplify a lot the current "pipeline" of escaping/(un)quoting that Dose do before sysexec'ing the command.

[danilobellini]

Historically, the idea of this interface without requiring the -- nor anything alike comes from the Linux watch command, which repeats a given command every N seconds, and defaults to N=2 when calling watch command (can be called with watch -n N command for any N). Before this project, we were using watch py.test and stuff alike during Coding Dojos, and I wanted to keep that interface.

The watch command even accepts commands with pipes. This:

watch "ls -l | awk '{print \$1}'"

should behave like this:

dose "ls -l | awk '{print \$1}'"

Calling watch echo -n 1 string works fine, the parameterless option -n is the one used by echo, this is different from calling watch -n 1 echo 1 string (doing so every 1 second instead of 2). I mean, the -n as part of the command argument isn't seen as an option to watch, but as part of the command itself.

A better API would be the one where the -- options-arguments separator is optional, and something as similar as possible to the watch interface.

AFAIK click can also do everything we're talking about here. I wrote this:

import click

@click.command(context_settings={
  "allow_interspersed_args": False,
  "ignore_unknown_options": True,
  "allow_extra_args": True,
})
@click.option("-n", nargs=1, default=2)
@click.argument("command", nargs=-1, type=click.UNPROCESSED)
def main(n, command):
    print("n = {n}\ncommand = {command}".format(**locals()))

if __name__ == "__main__":
    main()

You can test it with these:

python example.py echo -n 1
python example.py -- echo -n 1
python example.py -n 5 echo -n 1
python example.py -n 5 -- echo -n 1

This makes the -- optional and "parses" every case like the watch interface. The command should be quoted before joining back to a single command, unless it's a single parameter (where it could be a piped value, so there's nothing to quote there). That doesn't allow a command name with a whitespace and without options/arguments (unless we quote it twice), but I'm not sure on how watch would behave in this case.

The --test would be useful only for shell-specific commands like the ones that uses pipes, as a terminator (like the \; in find -exec command and its options/arguments \;), or an enforced quoting (e.g. dose --test 'py.test -vv' instead of dose py.test -vv) would be required all the time, and both are undesired IMO. Using it as a replacement for what exists today would be worse: the idea of having it as an alternative option was to to separate the single-parameter test command that shouldn't be quoted from a new "single-parameter test command that should be quoted", something that would be useful to make it work with optionless argumentless commands whose executable has a whitespace or anything a shell expands in its name, but I think we should drop out the clutter as much as we can. For this strange case of a single optionless argumentless whitespacey command, quoting it twice is already enough (e.g. dose '"my command name with whitespace"' on Linux/MacOSX/Cygwin, or dose "\"my command name with whitespace\"" on Linux/MacOSX/Cygwin/Windows) in both Dose and watch, isn't it?

I withdraw the --test idea. We should keep the watch interface on Dose as much as we can, so -- should become optional instead of dropping the current syntax.

[samuelgrigolato]

Ok, seems like we need not, and should not, worry about this too much. Based on your snippet I came up with this trimmed version:

[...]
@click.command(context_settings={
    "allow_interspersed_args": False
})
@click.argument('test', nargs=-1, required=False, type=click.UNPROCESSED)
def main(test):
[...]

This seems like to do the trick. One thing that I'm afraid is that AFAIK there isn't a comprehensive list of test cases for dose, one that summarizes all corner cases. Maybe writing such a doc is a good opportunity for improvement? I think I would be able to identify some of these problems by myself if one were available (as mainly a Windows developer I found myself seldom thinking about pipes and other command line idioms) What do you think?

So, I'm closing this issue, at least for now, with this conclusions:

• dose will not break compatibility by any stretch as a result of introducing cli-args.
• dose will not, for now, add a --test option, as it seems unnecessary.

Please feel free to correct me if necessary.