CLI task arguments

[bitprophet]

See also: Fabric #69

Problem

Need to allow parameterization of tasks as invoked via the CLI (tasks invoked via Python are simply passing regular args/kwargs or their list/dict counterparts.) Sub-problem: translating strings/flag existence into Python types such as lists, booleans etc.

Current implementation in Fabric is fab taskname:positional_arg_value,keyword_arg=kwarg_value,.... Some other tools use "regular" CLI flags, e.g. paver taskname -F value.

"Custom"/"attached" args

Pluses:

• Works great for multi-task invokes, zero ambiguity about which param values go to which tasks
• Supports positional arguments well

Minuses:

• Requires custom parsing, which may lead to bugs and at the very least increases code maintenance burden
• Slight documentation burden -- requires users to learn how that custom parsing works, re: need for escaping (esp. how that meshes with normal shell escapes) and etc.

"Regular" flag args

Pluses:

• Can (probably) just use argparse
• Many users already familiar with it

Minuses:

• Doesn't mesh well with multi-task invokes unless we:
  • enforce BSD-style "order matters" (not even sure argparse can do this? EDIT: perhaps with subcommands, if >1 subcommand can be invoked at a time)
  • or use an ugly "task delimiter" syntax like invoke task1 --arg1 --next task2 --arg2 (again, can argparse do this?)
  • or hope/pray that all simultaneously-invoked tasks have zero overlap in their kwargs and no positional args are ever needed.
    • How common is this? If we assume flags nix any possibility of positional args and that this is OK, it could be reasonable to assume two or three tasks called together would all have different names for their kwargs. Maybe.
• Namespace clashes with core (non task related) flags such as -c/--collection
  • This also requires order-matters, or simply limiting the user-allowed namespace
  • Limiting the namespace also presents backwards compatibility problems -- if version 1.1 lacks a --foo flag and a user declares mytask(foo='bar'), and then we unknowingly add --foo to core in 1.2, that user's code will break.
• May require more setup on the user side, re: explicitly declaring each task's args/kwargs via decorators or whatnot
  • Though we could introspect the function signature and automatically create some, if needed
• Doesn't allow for positional arguments unless we go the delimiter route.

Potential solutions

• Continue using Fabric's "custom parsing" solution pretty much as-is
  • See above re: pluses/minuses
• Try coming up with a "better" custom parsing solution/syntax, if there are any
  • What do other tools like Make, Rake, Thor etc use?
    • Thor uses regular flags + positional arguments, seems to simply ignore the "multiple tasks vs one task + positional args" problem, and uses "pass all flags to all tasks" to handle multi-task invocation.
      • Don't like this very much -- it "works" but has the "all tasks invoked must have different args" requirement mentioned above.
    • make uses shell environment variables, which is similar to how Thor treats flags -- every task is going to get the same values for any overlapping names.
    • Rake uses custom parsing like we do, but uses taskname[args, here] instead of taskname:args,here. I find this actually worse than the colon we use, because some shells like zsh treat square-brackets specially and need extra escaping/quoting.
• Allow custom parsing everywhere, but also allow use of flag args in single-task invocations
  • Plus: gives flag lovers what they want
  • Minus: "more than one way to do it" and its problems (newbie confusion, more code/bugs, more docs to write.)
• Use flags 100% of the time, relying on ordering or delimiting flags to handle multi-task invocations
  • See above re: pluses/minuses
  • Possibly use argparse subcommands, but AFAIK those are again targeted at using a single subcommand per invoke.

[bitprophet]

Iteration time:

• A few folks on IRC have mentioned they dislike the custom style for the reasons I anticipated and have heard before -- it's just different than the usual flags, is a bit more of a pain to type, etc.
• The 'delimit' sub-solution to using flags (or another special syntax, though all the non-alpha characters seem like they have special shell meaning already) feels like a dead end -- I'm not even sure argparse can handle it, and since it already implicitly requires "order matters", we may as well just go with the implicit version. (Again, if argparse lets us.)
• Subcommands are right out, argparse definitely cannot do >1 per invoke.

Still reading over argparse to see just what it gives us -- would prefer to work within it instead of doing something totally custom (which would defeat one of the reasons to go with flags anyways.)

[bitprophet]

Looks like argparse can't do interleaved "positional arguments" (i.e. task names) and options (flags), by default.

May be able to hack something together with one of:

• custom action objects
• manually preprocessing sys.argv into core args and a collection of tasks+their args, and using a bunch of "sub" parsers
• screwing around with the '*' version of the nargs action (though this feels like the inverse of what we need, it lets you associated multiple positional values to a given flag -- we need multiple flags associated with a "positional" task name argument.)

[bitprophet]

Custom action objects seem to only work if we knew beforehand how many "options" (non-flag values) were going to be passed in (as we'd need to call add_argument for each "slot"), in which case we'd have to parse argv ourselves anyways.

Though perhaps we could simply add all discovered tasks explicitly, as optional or something, to the parser. Slightly silly but might get the job done.

Will triple check this approach and how nargs works. Might be worthwhile to do the preprocess approach -- I can't think of any real edge cases we need that would make "split things up by not-a-flag strings and optional trailing --, pass each chunk into different parser objects" not work.

Part of me wants to use PyParsing for that, but not sure it's worth the dependency.

[bitprophet]

Right. Preprocessing isn't going to work because it's impossible to distinguish a non-equals-signed flag argument (--foo bar) from a task name, especially in more subtle cases (e.g. if we ever needed to use nargs), without literally reinventing what argparse does. So AFAICT there's no foolproof way to chop things up -- argparse simply doesn't map well to what we're trying to do here.

---

We could try the custom-action route linked above, relying on how the actions get called in order to reconstruct values -- this gives us all the "per argument/flag" parsing (--foo=bar vs --foo bar vs --foo I am using nargs lol) and would let us reconstruct a richer setup, but it has the following problems:

• Aforementioned need to add in all tasks + their arguments to the parser so it actually calls the action on them
  • N.B. optional and not actioned-upon unless actually given, positional args, work best as: parser.add_argument('label', default=argparse.SUPPRESS, nargs='?', action=MyActionClass).
    • MyActionClass only gets called if the task exists (so if you give it 10 and call 2, you only see the action called 2 times)
    • No errors about "too few arguments" when you tell the parser it could accept up to N positional args and <N are given (which would be the case 99.999% of the time, nobody calls their entire fabfile in one go.)
• Have to avoid adding the same flag twice for 2 different tasks, or use ArgumentParser(conflict_handler='resolve') to make it STFU.

---

Theoretically we could go whole hog and just say "fuck it" to argparse and use PyParsing instead, but I'll only do that if bending argparse to my will fails or turns into a huge mess-o-code. And if I still think it's worth it to not use Fab-style tasks.

[bitprophet]

• PyParsing probably wouldn't work after all -- you need the knowledge about whether a given --flag is supposed to take an argument after it (or multiple arguments, or whatever) in order to parse -- parsing a command string with none of that knowledge is impossible. Thus why argparse, etc exist and require use of add_argument.
  • Though we do want to add the type conversion feature eventually -- so if we had that set up, then we could use that information to tell which flags would require arguments and which would be just boolean switches. Feels like even more wheel reinvention, but meh.
• The "messed up argparse" approach is not working -- no matter what setup I use it seems to misinterpret what I want or skip parsing some args or values. There's no way this is going to end well.

[bitprophet]

@kennethreitz had one new spin on the "custom syntax to denote tasks apart" solution, which was to 'decorate' the task names in a custom manner, e.g. invoke task1: arg arg arg task2: arg arg arg.

Benefits: aesthetically pleasing (much nicer than --task=task1 --task=task2), sidesteps the problem of telling flag arguments apart from task names (and thus also allows positional arguments).

Drawbacks: falls into the "custom, non-traditional cli syntax" trap, possibly more confusing to use (what should happen if the colons are omitted? what does invoke task1 task2 do? heck, what does invoke task1 task2: args do? etc) and/or implement.

---

If we take "it's not going to look exactly like traditional argparse style" for granted, then we could also return to the idea of using some non-special-shell-meaning character to delimit task+arg chunks. E.g. invoke task1 args --flags - task2 or invoke task1 args --flags , task2. However, none of the available characters look all that great to me, or -- while not technically special to the shell -- have implications (like - implying that it is a regular arg value meaning to use stdin.)

[bitprophet]

Wrote up a shit ton of thoughts on this yesterday/today: http://docs.pyinvoke.org/en/latest/concepts/cli.html

For now, I am leaning towards the task: version because while it's less orthodox it should (I think) be much easier to parse, also also enables positional arguments, both of which would be kinda nice. We'll see what happens.

[tswicegood]

I think this could be done by chunking up sys.argv ourselves and passing only parts of it in at a time. It would require multiple passes, but would end up with a cleaner interface.

My initial thought is something like:

• Parse for a very small subset of args and determine all tasks.
• Break sys.argv on each task and assume everything after task A and before task B is supposed to go with task A
• Parse each list of arguments for each task.

It wouldn't be pretty and requires at least two trips through argparse, but it would allow invoke --global taskA --some-opt some-opt-value taskB --other --opt value style approaches. We should encourage the use of --opt=value to avoid ambiguous task clashing.

[tswicegood]

FWIW, I'll try taking a stab at the parser and see what I can come up with.

[bitprophet]

We should encourage the use of --opt=value to avoid ambiguous task clashing.

Thanks, I knew I'd forgotten at least one of the alternative approaches. Just added to the doc.

I'm not a fan of forcing users to do this, I feel it's too much of a departure from the "orthodox" style people are used to, even though it would simplify parsing. (Rather, it departs from regular argparse style while looking like it, which is IMO more confusing than being obviously something custom like Fab 1.x style.)

---

Re: the rest of your comment: if we went the "task names have no special decorations" route, we'd pretty much have to A) obtain knowledge of which flags need arguments and B) walk over sys.argv as you said, keeping our own state. (This is the "deep knowledge" methodology I mentioned in the linked docs.)

I think it's quite possible, but we can't use argparse for it unless I missed something seriously big when I spent a day or two the other week fucking around :( I'm also not sure there's any point, in this scenario, to try and use argparse for per-task bits, because just to get to the point where we have each task and its args split apart, we've already had to parse everything. There's nothing left for argparse to do.

I think the only use of argparse possible is if we went with one of the "easy to tell task chunks apart" routes (eg task: or the no-spaces-in-flags route), used one simple manual parse over argv to break things into chunks, then fed each chunk into its own argparse parser. I am actually leaning towards this approach right now, as per earlier comment.

[myusuf3]

👍

[bitprophet]

@tswicegood BTW I was hoping to do at least a proof of concept parser myself tonight / this weekend =( probably trying the colon-decorated approach first FWIW.

[bitprophet]

@myusuf3 (or any other users) do you have any thoughts to share? Specifically, choosing between this style:

$ invoke task1 --no-positional-args-here --kwargs are_ok_though task2

where you can't do positional arguments, but the task names have no special formatting; and this style:

$ invoke task1: positional args --allowed here task2: --task2-args

where you have to do the funky task1: but you can throw in regular non-keyword arguments.

Does the 2nd one look OK, dumb, other? Would you be sad to lose positional arguments if we did the first approach? etc.

[myusuf3]

@bitprophet I like the second one better simply because you need to have positional args. It looks kind of weird when you first look at it; but I can see it easily becoming second nature after a bit of use.

maybe embracing something like task1 | would be worth investigating as a potential replacement for task1:

if majority still finds it funky looking.

[dhellmann]

I've been doing some work in a CLI framework where I want to discover subcommands implemented in plugins, without loading the entire plugin set into the process space just to determine the names of the commands. I found optparse's ability to stop when it encountered a non-flag argument (a value that did not start with "-") helpful. Maybe you should be using that instead of argparse? https://github.com/dhellmann/cliff/blob/master/cliff/app.py#L36

[bitprophet]

@myusuf3 Well, the problem with characters like pipes (|) is they have special meaning for most shells. As far as I know none of the characters available to us that aren't shell-special, work too well besides the colon, and I personally think a character "attached" to the task name looks better than having a character "floating" out between tasks.

Though that brings me to another observation I made today, when we remove flags/args from the equation, these 'decorated't tasks look kind of silly:

$ invoke task1: task2: task3:

It would probably be possible to have context awareness so that the colons may be omitted, but that might complicate both parsing and usability.

---

@dhellmann I thought argparse was a superset of optparse's functionality? (also, I just got bitten by github defaulting to master -- I think you meant to link to this version of the file?) But if it works the way I think you're saying, it might be possible to run optparse or argparse in a loop and get the sort of result we need here.

I'll double check the differences between the two libs, I'm 99% sure argparse just wasn't able to do what I needed, but it's possible I missed something or optparse really does work differently. Thanks for the tip!

[kennethreitz]

As long as it only pays attention to trailing colons, it should be alright. I can't think of any scenario where someone would want them.

If they really want to get around it, they can just inspect sys.argv :)

[dhellmann]

@bitprophet Oops, yes, I should have linked to the specific file since I had changes to push. :-)

argparse is not a strict superset of optparse. It does not, for example, include the ability to do what you want here. The method parse_known_args() has slightly different semantics, and there is no disable_interspersed_args() as in optparse. With disable_interspersed_args() set, when you encounter:

task1 --opt value --opt value task2 --opt value --opt value

optparse will consume "task1 --opt value --opt value" and give you "task2 --opt value --opt value" in a list of arguments it did not consume. So it is very easy to run a loop, as you describe, and you don't need any special marker characters to tell the parser when to stop. That said, it would make it awkward to use positional arguments, because they would all need to appear before the options for a given task.

task1 positional --opt value --opt value task2 positional positional --opt value --opt value

[myusuf3]

I really do like the fact that it just results in a list of arguments that it didn't consume which you could iterate over till its completely exhausted.

@dhellmann the positional arguments before the options is awkward, but compiling things with gcc has made me accustomed to that syntax.

The problem is that you would still need to look for tasks names; which is burden on maintainers.

[tswicegood]

I've written up a quick example of what I was talking about last night and posted it here: https://gist.github.com/2437861

With this style, each task would get a subset of args to parse. This does have the problem of arguments for one task that are named the same as a task that you intend to invoke. I'm ok with that as it can be worked around by using --foo=ambiguous_name instead of --foo ambiguous_name.

I'm -1 on the task1:, task2: style of naming. I think Fabric's argument parsing has proven that deviating from expected behavior at all increases user friction. Granted, adding a colon isn't as big of a shift as learning an entirely new argument syntax, but I still don't think it's going to be what people expect.

[kmike]

I like special syntax with colons more because options written as

invoke --global taskA --some-opt some-opt-value taskB --other --opt value

looks like regular argparse/getopt arguments but have non-standard meaning (and imho this is worse than non-standard syntax + non-standard meaning).

[tswicegood]

@kmike for those of us on BSD, we're already used to having to pay attention to how we place args anyhow. :-)

[bitprophet]

I'm going to write tests so we can put parser solutions through their paces (or at least so edge/corner cases can be documented.) Would have to tweak to handle colon based vs regular style, but no big.

---

@kmike We can't have 100% "normal" invocation, it doesn't mesh with the idea of multiple tasks. But having an otherwise normal syntax with an "order matters" limitation -- a la BSD as Travis points out, which isn't uncommon -- may be the next best thing.

What's important to folks unhappy with Fabric 1.x's custom style, is that they can use flags and flag args: --flag, -f, -f arg, --flag arg, --flag=arg etc. The style under discussion (colon or no colon) enables that.

I agree that "Y looks like X but has subtle and frustrating differences" can be a killer, and that's why I don't want to cut corners in the flag parsing itself, e.g. to make --flag flag_arg impossible by requiring --flag=flag_arg.

---

@tswicegood Thanks for hacking on this. I acknowledge that --flag value_which_is_also_a_task_name is unlikely but I'd still love to solve it somehow (as per above.)

We could apply the "deep knowledge" stuff I mentioned earlier: encounter potential task name; look at preceding flag, if any; ask currently-being-parsed task if its spec for that flag says it should take an argument; if so, treat the string as an argument and keep going; if not, it's a task name. That sort of thing.

---

@dhellmann Re: optparse, my worry now is that even with a stop-start approach as described, optparse still thinks about things globally, no? That would gum up any situation where tasks have same-named kwargs. For example:

def task1(foo=True):
    pass

def task2(foo='bar'):
    pass

and invocation:

$ invoke task1 --foo task2 --foo=biz

Optparse, upon encountering task1 --foo task2, will not know which --foo this is, and so won't know if task2 is a positional arg (to stop parsing at) or the value to --foo.

I think that's moot anyways because optparse won't even let us define the same flag twice, AFAIK. Its conflict resolution feature doesn't seem to help there either.

The obvious resolution (enforce globally unique kwarg names) feels unacceptable to me.

[tswicegood]

@bitprophet What is the desired outcome. Do you want to be able to parse this:

$ invoke task1 --foo task2 task2

and get:

{
    'task1': {'foo': 'task2'},
    'task2': {},
}

I think to do that you need a @task decorator that looks like this:

@task(foo=arg("string"))
def task1():
    pass

@task 
def task2():
    pass

That kind of recognition means we're writing our own optparse/argparse, or something very similar to it. The algorithm (off the top of my head) would look like:

def parse(args):
    current_task = None
    for arg in args:
        if not current_task and is_task(arg):
            current_task = arg
            continue
        try:
            current_task.args.append(current_task.parse_arg(arg))
        except UnhandledArgumentException:
            if not is_task(arg):
                raise OMGWhatHaveYouGivenMe()
            current_task = arg

That's not as pretty as the "task names and options can't be the same w/o an equal sign" code I had above, but it could be made to work. That said, I do like the idea of the task being the final arbiter of what it can and can't handle. Assuming @task returns a Task object, it becomes trivial for someone to implement custom behavior by overriding parse_arg on the task.

[bitprophet]

@tswicegood yes, that's what I've been referring to as the "knowledge" approach, as per the first bullet point here (linked it on Twitter over the weekend, not sure if you saw it though, think I cc'd you).

Yea, it means we have to reinvent parts of optparse/argparse, which is unfortunate but I don't see a great way around it, and it would tie in with what we'd have to do re: implicit-or-explicit type casting as well.

As the other thoughts on that page highlight, the other main approach is to have an unambiguous "this is a task identifier" schema instead, which would let us break apart the invocation and then theoretically use actual-factual arg/optparse to handle the individual per-task chunks. That's @kennethreitz's idea with the trailing colon syntax.

[dhellmann]

@bitprophet It would be nice if this could be developed as an enhancement to argparse that went back into the stdlib.

[tswicegood]

@dhellmann +1, but I think we should play with it as a module inside invoke or maybe even a project by itself. The biggest problem (at least that I can think of right now) is how do you generically add something like the arg method I had in my pseudo-code above?

[bitprophet]

@dhellmann I'm not sure it would work -- this behavior strikes me as a fundamental change to the argparse/optparse model's core assumptions re CLI apps, and AFAIK would need to be implemented as a "make everything behave differently!" switch. But if I'm wrong and there's a way to make it fit, I'd certainly be +1.

I don't have the energy to try developing/forcing a sea change in a stdlib module, especially since I need these features for 2.6+ (and sooner instead of later), but I am in favor of the idea at least.

As per @tswicegood my plan is to have this be part of Invoke or maybe even another split-off lib that basically says "I'll do sorta argparse-like stuff for your CLI, but with this different focus." Reconciling that with argparse proper could run on its own schedule.

[dhellmann]

@bitprophet It seems like you just need a way to say "this group of arguments may repeat and should be processed separately". Something similar to the mutually exclusive group, but for repetition instead.

[bitprophet]

@dhellmann Oh yea, I guess that's true if we assumed somebody wanting this sort of multi task approach was going to use that "allow duplicate flags" feature in tandem with the "progressive consumption" approach discussed earlier. I still don't have the time to dive into their code now, but it's definitely something to think about for later.

[bitprophet]

Have been bad about linking work to this ticket, but have some basic work done at this point, test-driven, and now using the Fluidity state machine library to help with the actual parse loop (a literal for loop was too brittle re: cleaning up trailing state.)

Working: boolean flags, flags that take args, flag default values, multiple tasks/contexts per invoke.

Not working: handling --foo=bar (vs --foo bar), 'remainder' support (invoke blah blah -- this here should be one literal string), and hooking up the parser to the actual task execution (though this should be easyish as both ends of that equation are already working.)

[bitprophet]

Really at the crux of it now. Stuff being dealt with:

• Turning the foo in def mytask(foo): pass into --foo automatically
• Going from there to allowing optional short flags
• But once that's done it's time to wrap this up and move on to CLI arg typecasting #9 and others.

[bitprophet]

Got that working, then realized it poses a problem: right now Arguments have N names, intended to be potential parser matches, via a dict interface in the parent Context object (e.g. if <token seen in parsing> in <Context object>.)

So they need to be exactly what is seen on the CLI -- and so the test I just committed creates an Argument named --foo so that invoke --foo works.

Unfortunately, this same dict interface -- Context.args[argname] -- is intended to be used (And is used in tests) for accessing seen values post-parsing. At that point, we now have -- added in to everything. Not super handy.

---

Clear answer seems to be for the Context to expose a slightly different API to the Parser than it does to the clients of the parser result. What exactly that should look like, I'm not sure.

Alternately, we could simply add more aliases to Context's dict, so that it has both --foo and foo. However this A) muddies things and B) introduces ambiguity problems where mytask foo becomes valid, when it really shouldn't be (at least according to our spec.)

---

Brainstorming:

• The crux point here, given a "specify and query Arguments as non-flaglike string names" desired API, is the point where Parser queries Context objects to see if they exhibit the flag/token being handled.
• The naive approach, have it query for "--" + token, only fits the base case of simple long flags, and breaks as soon as we introduce short flags.
• So the knowledge about "is --flagname a valid flag" / "give me the Argument for --flagname" really needs to live inside the Context or below.
• So Context must expose a 2nd API besides .args, which is mostly for use by Parser, while .args is for use by clients of the result.
  • Though we can of course change how clients interact with the resulting Context objects too -- tho right now a Lexicon works well because it lets us preserve the common result.optionname access pattern.

[bitprophet]

Sweet, got that working, went with .args vs .flags, whose values are the same Argument objects, so the parser can manipulate .flags and the creators/consumers of Context objects can use .args for easier/more natural access.

Two more rough spots in the API and I think we can close this:

• Untyped arguments' default type. Right now I assume boolean, but that's not very friendly and breaks from all other parsing libs, which assume string. Change it to assume string, which will necessitate changing a lot of tests.
• Default values. I think this is just an oversight/untested spot, but e.g. a boolean Argument, when not actually flipped to True by being seen, will have a .value of None instead of False.
  • I forget if this is also the case with string args, though at least in that case None could be a valid default.
  • Could be as simple as changing it just for bools, then, and probably wasn't caught because None does evaluate to False when used as a test expression.

[bitprophet]

First bullet point done, plus related updates such that Argument kind values now pull from the default values in the task function, if available.

Re: the latter, I think None is a decent default value for the time being -- can change later if real world use dictates it.

[bitprophet]

Fleshed out more tests. Ran into one that should already work, but doesn't: invoke task1 -a value task2. Result from parser only has one context, I assume task1 but haven't debugged yet.

This is in the integration level tests that use a Collection, its to_contexts and a Parser.

Pretty sure I have passing tests in Parser that handle multiple Contexts. On the other hand, I verified that this Collection's to_contexts is correctly returning two Context objects, so the Parser is what's dropping/not seeing one of them.

---

After this, an area that has not been accounted for yet in the parsing (uh oh!) -- non-space short flags, and combo short flags. The parse engine up til now has assumed space delimited tokens so this will be an interesting challenge.

[bitprophet]

This is all set, what's left are some smaller skipped tests I'll fill in as we go.