Re: [PATCH v2 1/2] Introduce config variable "diff.primer"

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Jeff King
Date: Friday, February 13, 2009 - 3:22 pm

On Mon, Feb 09, 2009 at 09:24:37AM -0800, Keith Cascio wrote:


Good point.  I think there are actually two separate problems you
describe:

  1. some default behaviors which can be changed via an option have no
     option that represents the default. This is your "-b" example (and
     there are others, like "-a", "--full-index", etc).

     Generally I think we try to allow boolean options to be specified
     in either positive or negative ways. Our parse-options library even
     automatically handles --no-$foo by default for any boolean option
     (as long as it has a corresponding long option).

     So I consider the lack of --no-ignore-space-change to be a failing
     of git, but one that we can correct. Either manually or by moving
     the diff code to parse-options.

  2. For options which take a value, there is no way to say "pretend I
     didn't specify a value at all".

     Actually, that is not entirely true. parse-options handles
     "--foo=bar --no-foo" as if "--foo" was never specified at all.

     But there are still two failure cases:

       - as above, the diff options are not handled by parse-options

       - not all value options are quite as straightforward. Some
         options run callbacks that do things which take specialized
         code to undo.

     Both are fixable. But I have to wonder if it is really all that
     useful to say "use the default for this option". Either you don't
     care what the value is, in which case you can take the default
     given by your primer value. Or you do care, in which case wouldn't
     you want to be setting the value yourself?

So I think doing it right is a bit more work in the long run, but the
extra work is generally improving git.

All that being said, though, I still think we can do the equivalent of
--no-primer. The trick to avoiding multiple passes is for the option to
exist outside of the set of primer'd options. I can think of two places:

  1. an environment variable, GIT_PRIMER. E.g., "GIT_PRIMER=0 git diff".
     This strikes me as hack-ish and unfriendly, but would work.

  2. in the git option list, but not the git command option list. IOW, we
     actually have two sets of options in any command:

        git [options] diff [options]

     So you could suppress primers (all of them, including diff.primer
     or primer.*) via:

       git --no-primer diff

Make sense?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH v2 1/2] Introduce config variable "diff.primer", Jeff King, (Fri Feb 13, 3:22 pm)
[PATCH] Allow setting default diff options via diff.defaul ..., Johannes Schindelin, (Fri Mar 20, 8:15 pm)
Re: [PATCH] Allow setting default diff options via diff.de ..., Johannes Schindelin, (Thu Apr 9, 1:29 am)
Re: [PATCH] Allow setting default diff options via diff.de ..., Johannes Schindelin, (Thu Apr 9, 1:45 am)
Re: [PATCH] Allow setting default diff options via diff.de ..., Johannes Schindelin, (Thu Apr 9, 3:43 am)
[PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Mon Apr 13, 3:37 pm)
Re: [PATCH] Add the diff option --no-defaults, Jeff King, (Thu Apr 16, 1:34 am)
Re: [PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Thu Apr 16, 2:25 am)
Re: [PATCH] Add the diff option --no-defaults, Jeff King, (Thu Apr 16, 2:41 am)
Re: [PATCH] Add the diff option --no-defaults, Junio C Hamano, (Thu Apr 16, 9:52 am)
Re: [PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Thu Apr 16, 10:36 am)
Re: [PATCH] Add the diff option --no-defaults, Jeff King, (Fri Apr 17, 4:54 am)
Re: [PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Fri Apr 17, 6:15 am)
Re: [PATCH] Add the diff option --no-defaults, Keith Cascio, (Sat Apr 18, 9:41 am)
Re: [PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Sat Apr 18, 10:40 am)
Re: [PATCH] Add the diff option --no-defaults, Keith Cascio, (Sat Apr 18, 1:32 pm)
Re: [PATCH] Add the diff option --no-defaults, Johannes Schindelin, (Sat Apr 18, 2:15 pm)