Re: 'git mailinfo' whitespace bug

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2010, 19:57 UTC
Message-ID: <7vocjhcacs.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100222151344.GK3062@redhat.com>

Don Zickus <dzickus@redhat.com> writes:

Show 7 quoted lines

>> But when is it correct to ltrim a header line?  It means we are going to
>> accept a header (or header-looking line in body) that is indented.  I
>> don't know why 87ab799 (builtin-mailinfo.c 2007-03-12) was coded that way.
>
> In regards to 87ab799, I just deleted and pasted it from the function
> handle_inbody_header (which you can see from that commit).  The original
> code for those lines came from ae448e3854d8b6e7e37aa88fa3917f5dd97f3210.

Thanks for clarifying. The one in ae448e3 (mailinfo: ignore blanks after in-body headers., 2006-06-17) was about removing blank lines before the in-body headers begin, and never about removing indentation of the first in-body header. Admittedly, it was also being lenient and skipped over lines that are not empty but all whitespaces, and if we apply the quoted patch we will be retroactively tightening the rule, but I somehow do not think people would care.

Show 17 quoted lines

>>  builtin-mailinfo.c |    3 +--
>>  t/t5100/msg0015    |    2 +-
>>  2 files changed, 2 insertions(+), 3 deletions(-)
>> 
>> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
>> index a50ac22..ce2ef6b 100644
>> --- a/builtin-mailinfo.c
>> +++ b/builtin-mailinfo.c
>> @@ -779,8 +779,7 @@ static int handle_commit_msg(struct strbuf *line)
>>  		return 0;
>>  
>>  	if (still_looking) {
>> -		strbuf_ltrim(line);
>> -		if (!line->len)
>> +		if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
>>  			return 0;
>>  	}

We probably could do something like

	if (still_looking) {
        	if (strspn(line->buf, " \t\n") == line->len)
			return 0;
	}

to keep people with blank but not empty lines before the first in-body header happy. I somehow don't think it is worth it, but what would I know...