Subject: Re: [PATCH v4 08/25] sequencer: completely revamp the "todo" script parsing

From: Junio C Hamano <[email-masked]>
Date: Mon, 17 Oct 2016 15:33:02 -0700

Johannes Schindelin <[email-masked]> writes:

> -	for (i = 1; *p; i++) {
> +	for (i = 1; *p; i++, p = next_p) {
>  		char *eol = strchrnul(p, '\n');
> -		commit = parse_insn_line(p, eol, opts);
> -		if (!commit)
> -			return error(_("Could not parse line %d."), i);
> -		next = commit_list_append(commit, next);
> -		p = *eol ? eol + 1 : eol;
> +
> +		next_p = *eol ? eol + 1 /* strip LF */ : eol;

This one was explained as "skip LF" in the previous round, and that
is more correct than "strip", I think.  The +1 here is not done to
"strip" the LF out of the end result, but to "skip" one to move to
the beginning of the next line.

The one in v3 08/25 decremented the pointer to denote the end of the
line with the explicit purpose of not including the CR in the end
result, which was explained as "skip CR", but it was stripping CR.
Correcting that explanation to "strip" was a right fix and I think
your v4 09/25 still has it, which is good.

Other than this unnecessary change since the previous round, I
didn't spot a difference in this step, which was already good.

Thanks.