docopt.rs
docopt.rs copied to clipboard
Remove seemingly unnecessary Repeat patterns
This should make the tests in PR #185 much faster.
This was the pattern before my change:
r"""Usage:
prog a b [-p <param>]...
prog a c [-p <type> <param>]...
"""
$ program a c -p bool 1 -p bool 0 -p bool 1
{"-p": 3, "<type>": ["bool", "bool", "bool"], "<param>": ["1", "0", "1"]}
Usages:
Sequence([PatAtom(Command("a")), PatAtom(Command("b")), Repeat(Optional([PatAtom(Short('p')), PatAtom(Positional("param"))]))])
Sequence([PatAtom(Command("a")), PatAtom(Command("c")), Repeat(Optional([Repeat(PatAtom(Short('p'))), PatAtom(Positional("type")), PatAtom(Positional("param"))]))])
The Repeat(Optional([Repeat(...
causes an explosion in the function states
. states
was being called 1,421,237 times, and producing a vector of 236,975 elements.
This is the pattern after:
Usages:
Sequence([PatAtom(Command("a")), PatAtom(Command("b")), Repeat(Optional([PatAtom(Short('p')), PatAtom(Positional("param"))]))])
Sequence([PatAtom(Command("a")), PatAtom(Command("c")), Repeat(Optional([PatAtom(Short('p')), PatAtom(Positional("type")), PatAtom(Positional("param"))]))])
states
is being called 7,843 times, and producing a vector of 2,296 elements.
I'm not sure why the removed code was ever written. The concept of "has_repeat" means that the short can possibly repeat under some context somewhere, which seems to have a completely separate meaning from the Repeat(...) pattern. Furthermore, this extra Repeat(...) pattern only affects the second sequence and not the first. That's because after the first sequence is processed, Short('p') is flagged as something that repeats, which affects only the second sequence.
Hmm, that code was added in a4ee9b
to support this feature: https://github.com/docopt/docopt/issues/275
I find it curious that the tests still pass, but I can't yet explain why I added that code in particular. I'll take a closer look when I get a chance.
Thank you for looking into this. :-)