On Tue, Jun 24, 2008 at 12:30:48AM +0000, Junio C Hamano wrote:
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
>
> > On Mon, 23 Jun 2008, Linus Torvalds wrote:
> >>
> >> Umm. Helloo, reality.. There are actually very few options that take a
> >> flag for their arguments. In particular, the option parsing we really
> >> _care_ about (revision parsing - see builtin-blame.c which is exactly
> >> where I wanted to convert things) very much DOES NOT.
> >
> > Actually, I guess "--default" does, but if you try to mix that up with (a)
> > a default head that starts with a dash and (b) git-blame, you're doing
> > something pretty odd.
> >
> > And yes, "-n" does too, but if you pass it negative numbers you get what
> > you deserve.
> >
> > The point being, we really _do_ have a real-life existing case for
> > PARSE_OPT_CONTINUE_ON_UNKNOWN, which is hard to handle any other way.
> > Currently you can literally do
> >
> > git blame --since=April -b Makefile
> >
> > and while it's a totally made-up example, it's one I've picked to show
> > exactly what does *not* work with my patch that started this whole thread.
> >
> > And guess what you need to fix it?
> >
> > If you guessed PARSE_OPT_CONTINUE_ON_UNKNOWN, you win a prize.
>
> With this on top of Pierre's series, and adding PARSE_OPT_SKIP_UNKNOWN to
> the obvious place in your patch, "blame --since=April -b Makefile" would
> work.
>
> -- >8 --
> Subject: [PATCH] parse-options: PARSE_OPT_SKIP_UNKNOWN
>
> ---
> parse-options.c | 14 ++++++++++----
> parse-options.h | 1 +
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 71a8056..9f1eb65 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -341,20 +341,26 @@ int parse_options(int argc, const char **argv, const struct option *options,
> struct parse_opt_ctx_t ctx;
>
> parse_options_start(&ctx, argc, argv, flags);
> +
> + again:
> switch (parse_options_step(&ctx, options, usagestr)) {
> case PARSE_OPT_HELP:
> exit(129);
> case PARSE_OPT_DONE:
> break;
> default: /* PARSE_OPT_UNKNOWN */
> - if (ctx.argv[0][1] == '-') {
> + if (flags & PARSE_OPT_KEEP_UNKNOWN) {
> + ctx.out[ctx.cpidx++] = ctx.argv[0];
Actually this doesn't work because it may point into the strbuf that
will be invalidated later. Our sole option here is to leak some memory I
fear.
--
·O· Pierre Habouzit
··O madcoder@xxxxxxxxxx
OOO http://www.madism.org
Attachment:
pgpRxxgeCTk9t.pgp
Description: PGP signature
