On Mon, Jun 23, 2008 at 11:47:49AM -0700, Linus Torvalds wrote:
> > $ git blame --default HEAD git.c
> > fatal: cannot stat path HEAD: No such file or directory
> >
> > Oops.
>
> Oops. And then, how would you fix this most easily?
>
> Be honest now.
Your statement is obviously loaded. I haven't seen _anything_ that fixes
it except what I have already mentioned. But I'm sure you are going to
complain that it isn't easy enough.
> Example: many arguments cause multiple option variables to change.
> parse_options() simply can't handle that well - you can do it with a
> callback, but then you need to make the option variables global or make
> them a structure or something. All of which just makes it nasty to do
> partial conversions for the simple cases.
I'm not so sure. I assumed that most of the callbacks would simply take
a "struct rev_list". So you would end up in builtin-blame.c with:
...
OPT__REVISION(&my_rev_list),
...
in your options table. And if setup_revisions takes options that affect
things that _aren't_ in that struct, then they probably ought to be.
> And I guarantee that just adding PARSE_OPT_{CONTINUE|STOP}_ON_UNKNOWN is
> going to be the smallest patch, and make for the easiest usage case. It
> may not be "pretty", but I can whip up a patch in five minutes.
I don't have a problem with STOP_ON_UNKNOWN, as I think it is a building
block upon which sane things can be done (like linearly going through
each parser and saying "did you want this?"). I think IGNORE/CONTINUE
has a fundamental flaw.
> Or are we going to sit around discussing this for another five months?
Please! :)
Pierre was working on the approach I mentioned, but I think he is short
on time. I will take a look at the conversion, but I have a few other
fixes on my plate first.
In the meantime, I don't think your patch makes anything _worse_, since
we already have these sorts of bugs in the current parsing code.
-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
