Bugfix for blank pattern among other subcommands

[Met48]

See test_bug() for an example of the issue.

[keleshev]

Isn't it a bit of a hack to manipulate printable usage, instead of how it is parsed? I was attempting to change parsing to fix this.

Otherwise we're short on 1 bug—that's great. I will pull when I get home.

[Met48]

I don't think so, as the issue results from how an Either being parsed will ignore empty options. This only prevents weird usage strings like this:

usage: prog (a|b|)

Fixing the parsing allows this as well.

[keleshev]

Maybe I don't understand you 100%, but why not allow (a|b|) to be equivalent to (a|b|()) and thus to [a|b] (this maybe gives some opportunity to refactor (eliminate?) Optional). I would consider changing how Either is parsed.

[Met48]

I think refactoring away Optional can be done easily actually - just update line 379 to create a Required containing an Either node for each result of the parse_expr (with an empty case as well), then remove the other few references.

My only concerns with a format like (a|b|) are that it is visually confusing to the user and could also indicate a programmer error. After all, [a|b] is equivalent and is by far a clearer way of expressing the intent. Another problematic expression is a | at the start / end of a line, as in Usage: prog add <name> |. This could occur while the programmer is refactoring the usage message. I think the best solution here would be to raise a parsing error if a user-created Either has an empty case, rather than have them surprised when it later does or doesn't accept the empty case.

As a side note, I thought of a much better approach with more consistent behaviour than this commit, so I'll be updating this pull request later today.

[Met48]

@halst I've updated the pull request to a lot more concise a solution. It also no longer alters the (undefined) behaviour with leading or trailing | characters. All tests pass except one which explicitly checks the formal usage and has since been updated.

[keleshev]

This one is good. I will say it's an elegant hack that fixes serious bug—so I pull this.