Re: [PATCH v2 1/5] core.aheadbehind: add new config setting

[Junio C Hamano <gitster@pobox.com>]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> Created core.aheadbehind config setting and core_ahead_behind
>> global variable.  This value defaults to true.
>>
>> This value will be used in the next few commits as the default value
>> for the --ahead-behind parameter.
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> ---
>>  Documentation/config.txt | 8 ++++++++
>>  cache.h                  | 1 +
>>  config.c                 | 5 +++++
>>  environment.c            | 1 +
>>  4 files changed, 15 insertions(+)
>
> Not a reason to reroll on its own, but this seems out of order: the
> series is easier to explain and easier to merge down in stages if the
> patch for --ahead-behind comes first, then the config setting.
>
> More generally, new commandline flags tend to be less controversial
> than new config settings since they cannot affect a script by mistake,
> and for that reason, they can go earlier in the series.
>
> As a bonus, that makes it possible to include tests.  It's probably
> worth adding a test or two for this new config setting.
>
> [...]
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 9593bfa..c78d6be 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -895,6 +895,14 @@ core.abbrev::
>>  	abbreviated object names to stay unique for some time.
>>  	The minimum length is 4.
>>  
>> +core.aheadbehind::
>> +	If true, tells commands like status and branch to print ahead and
>> +	behind counts for the branch relative to its upstream branch.
>> +	This computation may be very expensive when there is a great
>> +	distance between the two branches.  If false, these commands
>> +	only print that the two branches refer to different commits.
>> +	Defaults to true.
>
> This doesn't seem like a particularly core feature to me.  Should it be
> e.g. status.aheadbehind (even though it also affects "git branch") or
> even something like diff.aheadbehind?  I'm not sure.

FWIW, I do not think it is core at all, either; sorry for not
anticipating that a wrong name will be picked without a proper
guidance when I saw the "not limited to status" mentioned in the
discussion, but I was sick and offline for a few days, so...

> I also wonder if there's a way to achieve the same benefit without
> having it be configurable.  E.g. if a branch is way behind, couldn't
> we terminate the walk early to get the same bounded cost per branch
> without requiring configuration?

Hmm, that is an interesting thought.