On Sat, Mar 01, 2008 at 01:37:26AM -0800, Junio C Hamano wrote:
> > The behavior should be identical, except that we now support
> > option bundling (e.g., "git shortlog -nse").
>
> Sorry, but this breaks
>
> git shortlog -n -s -e --no-merges v1.5.4..
I didn't test this very extensively (obviously!) since it was just a
respin of an old patch. Sorry to waste your time with something that is
so obviously broken.
I started to write a patch to give parse-options a "stop processing at
the first unknown option" flag, but it was very unsatisfactory.
Specifically:
- the final parser has to know that it's the last, and complain about
unrecognized options. In this case, setup_revisions would be the
second and final parser, and it of course doesn't do this (though
perhaps one could check the residual options from setup_revisions
manually and barf on that)
- the fact that we have two sets of parsed options can't be
transparent to the user. We'd stop at "--no-merges", which means you
can't say "git shortlog --no-merges -e", which is silly. But we
can't possibly continue, since we don't know if "-e" is another
option or an argument there.
And it doesn't work to do the setup_revisions first, since there is a
conflict over the "-n" option (and we can't even munge the result
afterwards, since one version takes an argument and one doesn't).
So I think using parse-options here should be put on hold until we have
the revision and diff parameters in a parseopt-understandable form. I
would think we could do something like:
#define OPT__REVISION(x) \
OPT_BOOLEAN(0, "no-merges", &(x)->no_merges, "don't show merges"),
OPT_BOOLEAN(0, "boundary", &(x)->boundary, "show boundary commits"),
...
and we could have unified options tables. I seem to recall some work
being done in this area early on in the parse-options history, but I
can't seem to find any mention of it in the list archive. Pierre, does
this ring a bell?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
