Thread (218 messages) flat view 218 messages, 9 authors, 2016-11-06

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

From: Junio C Hamano <hidden>
Date: 2016-10-17 22:33:10

Johannes Schindelin [off-list ref] 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.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help