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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 2, 2018, 22:17 UTC
Message-ID: <20180102221717.GD131371@aiede.mtv.corp.google.com>
In-Reply-To: <2a5eb18a-d1b4-da2d-c2c8-5798d8627617@jeffhostetler.com>

Hi,

Jeff Hostetler wrote:

> On 12/21/2017 3:43 PM, Jonathan Nieder wrote:

Show 12 quoted lines

>> 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?
>
> I created a config setting because we don't want to force users to
> type "git status --no-ahead-behind" on every interactive command to
> get the benefit of it.  I guess we could ask them to alias it, if we
> don't want a config setting.
>
> Also, I didn't want to change the time-tested behavior that users see,
> so I didn't want to change the algorithm in any way -- just not call it.

I'm not too worried about people relying on the time-tested behavior in this case. It's a convenience feature for human users --- any scripts looking for this information would be likely to use a more convenient command like rev-list.

The one exception is "git status --porcelain=v2". For that command, scripts are likely to expect to be able to parse the "+<ahead> -<behind>" line. Alas, guarding it with a config doesn't help such scripts --- they still need to be able to cope with the new behavior.

git-status(1) says:

	Parsers should ignore headers that they don't recognize.

so introducing a new line like "# branch.matchesupstream (true | false)" like you did seems reasonable enough. I *suspect* it should be okay to omit the "# branch.ab" line as long as the script didn't explicitly pass --ahead-behind but that depends on whether any scripts in the wild were relying on the "# branch.ab" line being present.

> Would it make more sense to name this something like "status.aheadBehindLimit"
> where 0 would mean no limit and match existing behavior and a positive
> number be the number of commits we are allowed to search before giving up.

Sounds good to me.

Thanks, Jonathan