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

From: Jonathan Nieder <[email-masked]>
Date: Tue, 27 Jul 2010 16:09:08 -0500

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