Re: [PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 27, 2010, 21:09 UTC
Message-ID: <20100727210908.GA11317@burratino>
In-Reply-To: <1280261936.4462.6.camel@walleee>

Will Palmer wrote:

> the purpose of the patch was to respect --abbrev instead of always
> abbreviating to a minimum of 7 characters. /Not/ to respect abbrev
> "instead of always abbreviating".

Sure, though it had that added effect.

One goal of that series was to be able to write formats like this:

	%C(commit)commit %H%Creset
	%M(Merge: %p
	)Author: %an <%ae>
	Date:   %ad

	%w(0,4,4)%B

to replicate the effect of --format=medium. With diff-tree (and rev-list before v1.7.0.6~1^2) that is not possible if %p abbreviates by default.

Of course, v1.7.0.6~1^2 illustrates that no one seems to have been relying on the format of Merge: lines, anyway, so I am not saying that to make diff-tree --format=medium abbreviate by default would be a bad change.

> Perhaps armed with that phrasing, a
> more general solution, such as equating "0" with "DEFAULT_ABBREV" rather
> than "no abbrev", could be applied?

Maybe. If so, one would have to deal with the other callers that explicitly set abbrev to 0.

 probably just confusing no-ops:

 - bisect.c::bisect_rev_setup
 - bisect.c::show_diff_tree (to imitate diff-tree: probably a no-op
   because there is no setup_revisions call)

 means FULL_SHA1:

 - diff-files.c::cmd_diff_files
 - diff-index.c::cmd_diff_index
 - diff-tree.c:cmd_diff_tree
 - revision.c::handle_revision_opt

Hope that helps, Jonathan