Re: [RFC] Re: Convert 'git blame' to parse_options()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 23, 2008, 16:25 UTC
Message-ID: <alpine.DEB.1.00.0806231709540.6440@racer>
In-Reply-To: <20080623155315.GA18593@artemis.madism.org>

Hi,

On Mon, 23 Jun 2008, Pierre Habouzit wrote:

Show 31 quoted lines

> On Mon, Jun 23, 2008 at 12:26:41PM +0000, Johannes Schindelin wrote:
> 
> > On Mon, 23 Jun 2008, Pierre Habouzit wrote:
> > 
> > > This "PARSE_OPT_IGNORE_UNRECOGNIZED" thing has been discussed many 
> > > times in the past, but it just doesn't fly.
> > > 
> > > Though to help migrations we can probably introduce a new parse 
> > > option construct that would be a callback that is responsible for 
> > > dealing with "things" the upper level parser doesn't know about, 
> > > something where the callback could be:
> > > 
> > > enum {
> > >     FLAG_ERROR = -1,
> > >     FLAG_NOT_FOR_ME,
> > >     FLAG_IS_FOR_ME,
> > >     FLAG_AND_VALUE_ARE_FOR_ME,
> > > }
> > > 
> > > int (*parse_opt_unknown_cb)(int shortopt, const char *longopt,
> > >                             const char *value, void *priv);
> > 
> > I believe that this is what Junio was talking about when he mentioned 
> > callbacks.
> > 
> > However, I think it buys us more trouble than it saves us.
> > 
> > Thinking about the recursive approach again, I came up with this POC:
> 
>   Well I proposed something like that in the past, and we believed it to
> be too cumbersome.

IIRC your solution was a bit involved, while I think that mine is at least small enough to be simple.

Note: it is a bit wasteful, allocating space for a new option table 
in every case, even if there are no OPTION_OPTIONS, but I think that is 
okay.

Show 5 quoted lines

> I can live with it well fwiw, but it doesn't solve the issue of 
> migrating a very complex option parsing chain to parse-options well 
> IMHO. THe big problem with diff and rev opt parsing is that one you want 
> to migrate _any_ of the commands, you have to migrate _all_ of them, 
> which is huge.

I don't think you have to migrate all of them in one go.  To the contrary, 
if we have both "diff_opt_parse()" as well as a "struct options 
*diff__options", it can be done one by one.  During that time, though, 
people would have to add new diff options to both places.
 
Ciao,
Dscho