Re: [PATCH] Make find_commit_subject() more robust

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 22, 2016, 09:03 UTC
Message-ID: <alpine.DEB.2.20.1606221041320.10382@virtualbox>
In-Reply-To: <xmqqziqent0e.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 21 Jun 2016, Junio C Hamano wrote:

Show 16 quoted lines

> Your patch introduces a similar loop with similar (but different)
> purpose:
> 
> 		while (*p == '\n')
> 			p++;
> 
> which would have been helped if there were a helper with an
> opposite function, i.e.
> 
> 		p = strcchrnul(p, '\n');
> 
> i.e. "give me the pointer to the first byte that is not '\n', or EOS".
> 
> But there is no such thing.  Although p += strcspn(p, "\n") is a
> possibility, that somehow feels a bit odd.  And that is why I did
> not hint any existing function and said "might want to find".

Sure. And strcspn() is less efficient than the loop if you already know that the second parameter contains only a single character.

Show 8 quoted lines

> HOWEVER.
> 
> Stepping back a bit, I think what we actually want is
> 
> 		p = skip_blank_lines(p);
> 
> that skips any and all blank lines, including an empty line that
> consists of all whitespace.

My original aim was to make find_commit_subject() consistent with the pretty-printing machinery. I failed to realize that skip_blank_lines() does more than skipping empty lines, so let me re-roll the patch.

Ciao, Dscho