Re: [PATCH v2 3/3] commit::print_summary(): set rev_info.always_show_header to 1

[Tay Ray Chuan <rctay89@gmail.com>]

On Sat, May 29, 2010 at 9:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>> 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?

A cursory look at log-tree.c leads me to believe only show_log() cares
about log.parent.

In any case, if you look a few more lines up, you would notice another
log.parent = NULL:

@552
	struct log_info log;
	int shown;

	log.commit = commit;
	log.parent = NULL;
	opt->loginfo = &log;

	shown = log_tree_diff(opt, commit, &log);
	if (!shown && opt->loginfo && opt->always_show_header) {
		log.parent = NULL;
		show_log(opt);
		shown = 1;
	}

Therefore, I suspect that log.parent = NULL is a kind of
initialization for show_log() (log_tree_diff() does call show_log()
too).

>> +     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 ;-).

Based on my reading, I can't think of any.

-- 
Cheers,
Ray Chuan