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

From: Junio C Hamano <[email-masked]>
Date: Tue, 17 May 2011 11:50:47 -0700

Jay Soffian <[email-masked]> writes:

> @@ -137,6 +139,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>  	    && rev->commit_format == CMIT_FMT_RAW)
>  		decoration_style = 0;
>  
> +	/* ditto for log.abbrevCommit */
> +	if (!rev->abbrev_commit_given && rev->abbrev_commit
> +	    && rev->commit_format == CMIT_FMT_RAW)
> +		rev->abbrev_commit = 0;
> +
> +

This is not exactly "ditto"; the lines before this hunk look like this:

	/*
	 * defeat log.decorate configuration interacting with --pretty=raw
	 * from the command line.
	 */
	if (!decoration_given && rev->pretty_given
	    && rev->commit_format == CMIT_FMT_RAW)
		decoration_style = 0;

The check for pretty-given and commit-format being raw is to catch only
the case where the command line said --pretty=raw, excluding any case the
commit-format is set to raw without any explicit --pretty=raw on the
command line.  So at the logical level, this one should be read as if it
was written like this:

	if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {
        	/* "log --pretty=raw" is special */
        	if (!decoration_given)
			decoration_style = 0;
	}

even though the existing one makes it look as if checking decoration-given
and pretty-given together when commit-format is set to raw.

Care to explain why your check has to be different?  If there is no reason
to use a different logic, then a natural thing to do is to rewrite the
existing decoration logic and add yours like this:

	if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {
        	/* "log --pretty=raw" is special */
        	if (!decoration_given)
			decoration_style = 0;
		if (!abbrev_commit_given)
			rev->abbrev_commit = 0;
	}