Re: [PATCH v7] commit: add a commit.verbose config variable

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 15, 2016, 19:24 UTC
Message-ID: <CAPig+cTRWg9yB7eXhnbgbjjYfrJyn_A-_gQmbW8NL1DEbkLpkQ@mail.gmail.com>
In-Reply-To: <CAFZEwPNAVxT2HVQyj0Wajth_y-P6Af4AcSZDzJDJP=vLHztwiw@mail.gmail.com>

On Tue, Mar 15, 2016 at 3:00 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:

Show 15 quoted lines

> On Tue, Mar 15, 2016 at 5:01 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> You made 'commit.verbose' a boolean, so either verbose or not, ...
>> ... but note these context lines telling us that --verbose can be
>> specified not just once but twice (and who knows what the future may
>> bring).  This raises some not entirely rhetorical questions:
>>
>>   - What does 'git config commit.verbose true && git commit --verbose'
>>     do?
>
> This is a nice thought which didn't strike me!
>
> As Eric Sunshine mentioned ($gmane.org/288811), it would react
> according to the multiple verbosity level and since its not currently
> defined in `commit` it will react as it is reacting when verbosity
> level is 1.

I get the feeling that you missed SZEDER's point which was that git-commit already behaves differently when --verbose is specified multiple times. (I hadn't gotten around to reviewing that part of the code yet, so I'm glad that SZEDER saved me the effort.)

The new config variable, which is boolean, doesn't mesh well with multiple verbosity levels. For instance, with a combination of commit.verbose=true and a single --verbose, the code will think that --verbose was given twice and use verbosity level 2, which is not at all intuitive and would be surprising for the user. So, SZEDER was asking how this impedance mismatch can be rectified.

A possibly sane approach would be to make commit.verbose be a verbosity level rather than boolean, and behave as follows:

1. if --verbose is used (one or more), completely ignore commit.verbose.
2. else, if commit.verbose is set, use it.
3. else, default behavior.

I'm not sure if this makes sense, but as a convenience, maybe also recognize "true" and "false" as aliases for 1 and 0, respectively, for values of commit.verbose.

And, of course you'd want to test these behaviors.