Re: [PATCH v3] Add log.abbrevCommit config variable

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 16, 2011, 07:00 UTC
Message-ID: <20110516070047.GA26270@elie>
In-Reply-To: <BANLkTi=iFgJ12=_amoxT8x+hNMEhQtOVBg@mail.gmail.com>

Jay Soffian wrote:

> On Sun, May 15, 2011 at 6:42 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

>> I am not entirely happy about this change. The "ditto" refers to an ugly
>> workaround we had to add with 4f62c2b (log.decorate: only ignore it under
>> "log --pretty=raw", 2010-04-08) only because it was too late to revert the
>> change in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was
>> about to become part of 1.7.2-rc0 release. If we knew better, we probably
>> wouldn't have added the log.decorate variable that requires this hack that
>> requires us to say that 'log --pretty=raw' is special.
>>
>> If we stop before adding a new configuration, we do not have to repeat the
>> same mistake we made earlier.
>>
>> I dunno.
>
> I disagree that log.decorate is a mistake and that the workaround is
> ugly.

I suppose part of what Junio is saying is that by the time the commits referenced above were written, git had already broken some scripts (including gitk) and those changes were part of a desparate attempt to contain the damage. So they are not a great example to look to for the sort of smooth transition it is possible to set up proactively.

For example, maybe (after fixing the scripts we already know about, such as tig) we could add the log.abbrevcommit variable right away but advertise it as experimental:

	*Warning* This option is experimental and will break your
	scripts.  It is only provided to give script authors a
	chance to test this functionality and fix their scripts
	before the feature is advertised in earnest.

One transition plan could look like this:

 1. In the release notes to v1.7.6, mention that there is a change
    on the horizon that would break people's scripts and encourage
    script authors to switch to "rev-list | diff-tree -s --stdin"
    if their scripts depend on the details of "git log" format
    (in particular, if their scripts do not work correctly after
    s/log/log --abbrev-commit/).  Introduce the log.abbrevcommit
    variable to help people test, guarded by a compile-time
    option and disabled by default.

 2. In v1.7.7, introduce the log.abbrevcommit variable, advertised
    as "This will break your system --- don't use it unless you
    are trying to find such breakage and fix it".

 3. In v1.8.0, introduce the variable in earnest and recommend
    that people use it.

I think step 1 is going too far --- it should be possible to give users rope like this without worrying that they are going to be irresponsible about it.

Now, returning to "log --pretty=raw". Is it plumbing or not? It would be nice to advertise whichever way it is decided (I guess it is de facto plumbing) in the "git log" reference documentation and to follow that decision in cases like this one.

Thanks for some food for thought.

Ciao, Jonathan