Re: [PATCH 1/3] mailinfo.c: is_scissors_line ends on newline

From: Jeff King <peff@peff.net>
Date: May 12, 2017, 08:31 UTC
Message-ID: <20170512083131.nb5zyurmeg6n5auo@sigill.intra.peff.net>
In-Reply-To: <20170512050347.30765-2-bmalehorn@gmail.com>

On Thu, May 11, 2017 at 10:03:45PM -0700, Brian Malehorn wrote:

Show 7 quoted lines

> Needed to work with git interpret-trailers. Since "line" is, of course,
> a line, it will always end with "\n\0" and therefore we can safely end
> on "\n".
> [...]
> -	for (c = line; *c; c++) {
> +	for (c = line; *c != '\n'; c++) {
>  		if (isspace(*c)) {

Hrm. So I don't think this is _wrong_ per se, but the first thing we do in the loop is check isspace(), which a newline should match. And you can't see it from the diff context, but it always hits a continue, which would then advance to the NUL and exit the loop.

So I'm puzzled why this change is necessary. Am I missing something? We would increase the gap and perforation if we're in them, so I guess it could have an impact on the line-length heuristics that come at the end of the function. Is that it?

-Peff