From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 29 May 2010 01:10:52 GMT
Subject: Re: [PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1
Message-ID: <7vd3wf5x7n.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1274974492-4692-4-git-send-email-rctay89@gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

> This attempts to fix a regression in git-commit, where non-abbreviated
> SHA-1s were printed in the summary.
>
> One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the
> `if` block. However, we remove this codeblock altogether, and set
> rev.always_show_header.
>
> This way, we use back the same show_log() mechanism (instead of
> format_commit_message()).

I like the removal of the handcrafted call to f-c-m.  Thanks.

> Quoting log-tree.c:560:
>
> 	shown = log_tree_diff(opt, commit, &log);
> 	if (!shown && opt->loginfo && opt->always_show_header) {
> 		log.parent = NULL;
> 		show_log(opt);
> 		shown = 1;
> 	}
>
> This is the only area that always_show_header is checked, so the
> setting of this flag should only affect this area.

Hmm, but also setting this flag would affect anything that changes
behaviour depending on the value of log.parent, no?

> +	if (!log_tree_commit(&rev, commit))
> +		die("unable to print summary");

When always_show_header is set, what are the situations where
log_tree_commit() might return false?  I think your fix depends on the
fact that it will never return false (which I think is a correct thing to
assume---after all that is what "always_show" means ;-).