Re: [PATCH] blame: add blame.showemail config option

From: Quentin Neill <quentin.neill@gmail.com>
Date: Apr 30, 2015, 14:03 UTC
Message-ID: <CACfD1vtfO_rqr4wv4oHm7W7DSn-Gk-du6nCuJzYvpPXK_d=y=g@mail.gmail.com>
In-Reply-To: <xmqqmw1svigd.fsf@gitster.dls.corp.google.com>

On Mon, Apr 27, 2015 at 1:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

>>> It's not clear why you relocated documentation of --show-email from
>>>
>> I moved it to be with the other variables that had configuration
>>
>> options, but I will move it back.
>
> Please do not do anything before you understand why there are two
> places, and if you don't understand, ask.
>
>> I followed the code for blame.showRoot and blame.blankboundary.
>
> I do not think that is quite true.

Points well taken. I'll admit I posted the patch as a way to find out these sorts of things, but I see I could have done a bit more digging and asking before posting. Apologies for the noise.

On Tue, Apr 28, 2015 at 2:08 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>> .. instead set the OUTPUT_SHOW_EMAIL bit in git_blame_config().
>
> Don't forget to clear the bit when the bool is set to false, too.

I think I have this now, and some tests that check it, but have a pair of questions.

On Fri, Apr 24, 2015 at 12:22 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

> Despite being case-insensitive and despite existing inconsistencies,
> in documentation, it is customary to use camelCase for configuration
> options, so "blame.showEmail".

I noticed while testing that git_config()/git_value() lowercase everything, so to be clear this camelCase custom for configuration names is for documentation only, right?

I'm thinking of a test file that will test all the git blame options, but for this patch it will only test the new showEmail config. I read t/README and tentatively named the new test file "t/8009-blame-config.sh".

Please guide.