Re: [PATCH] Allow setting default diff options via diff.defaultOptions

From: Jeff King <peff@peff.net>
Date: Apr 10, 2009, 08:01 UTC
Message-ID: <20090410080155.GB32195@coredump.intra.peff.net>
In-Reply-To: <alpine.DEB.1.00.0904091242430.10279@pacific.mpi-cbg.de>

On Thu, Apr 09, 2009 at 12:43:28PM +0200, Johannes Schindelin wrote:

Show 13 quoted lines

> > > Now, it is easy to put a patch on top of my patch to support something 
> > > like --no-defaults.
> > 
> > No, it's not. We went over this in great detail earlier in the thread. 
> > If you want:
> > 
> >   git diff --no-defaults
> > 
> > then you basically have to parse twice to avoid the chicken-and-egg
> > problem.
> 
> So what?  We parse the config a gazillion times, and there we have to 
> access the _disk_.

But the first parse is only looking for "--no-defaults". So you need to:

  1. Understand the semantics of the other options to correctly parse
     around them (i.e., knowing which ones take arguments).

  2. Parse the arguments without actually respecting most of them, since
     they will be parsed again later.

This would actually be pretty easy if we had a declarative structure describing each option (like parseopt-ified options do). But the diff options are parsed by a big conditional statement.

Two ways to make it easier would be:

  1. You could loosen (1) above by assuming that --no-defaults will
     never appears as the argument to an option, and therefore any time
     you find it, it should be respected. Thus your first parse is just
     a simple loop looking for the option.

  2. You could loosen (2) above by assuming that all options are
     idempotent. I don't know whether this is the case (I think it
     isn't for all git options, but a cursory look shows that it may be
     for diff options).

-Peff