Re: [PATCH v3 1/2] sequencer: factor out parsing of todo commands
From: Junio C Hamano <hidden>
Date: 2026-06-22 17:00:27
Phillip Wood [off-list ref] writes:
From: Phillip Wood <redacted> Move the code that parses todo commands into a separate function so that it can be shared with "git status" in the next commit. As we know the input is NUL terminated we do not pass a pointer to the end of the line and instead test for a blank line by looking for NUL, CR LF, or LF. We use starts_with() instead of starts_with_mem() for the same reason. This results in slightly different behavior when there a CR at the start of the line that is not followed by LF. Previously such a line was treated as a comment rather than an invalid line.
Meaning that the input validation is tighter than before? I think it is fine in this case, as I do not see a reason why anybody wants to use a lone CR as comment introducer.
+bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd)
+{
+ const char *s = *p;
+
+ for (int i = 0; i < TODO_COMMENT; i++)
+ if (is_command(i, p)) {
+ *cmd = i;
+ return true;
+ }
+
+ if (starts_with(s, comment_line_str)) {
+ *cmd = TODO_COMMENT;
+ return true;
+ } else if (s[0] == '\n' || (s[0] == '\r' && s[1] == '\n') || !s[0]) {
+ *cmd = TODO_COMMENT;
+ return true;
+ }
+
+ return false;
}I notice that the order of noticing concrete comments and comment lines are swapped relative to the original. There is no inherently "natural" order between them, so the change is perfectly OK. I just got confused slightly while reading it until I realized that is what you did.
quoted hunk ↗ jump to hunk
static int check_label_or_ref_arg(enum todo_command command, const char *arg)@@ -2716,29 +2737,23 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts, { struct object_id commit_oid; char *end_of_object_name; - int i, saved, status, padding; + int saved, status, padding; item->flags = 0; /* left-trim */ bol += strspn(bol, " \t"); - if (bol == eol || *bol == '\r' || starts_with_mem(bol, eol - bol, comment_line_str)) { - item->command = TODO_COMMENT; - item->commit = NULL; - item->arg_offset = bol - buf; - item->arg_len = eol - bol; - return 0; - } - - for (i = 0; i < TODO_COMMENT; i++) - if (is_command(i, &bol)) { - item->command = i; - break; - } - if (i >= TODO_COMMENT) + if (!sequencer_parse_todo_command(&bol, &item->command)) return error(_("invalid command '%.*s'"), (int)strcspn(bol, " \t\r\n"), bol); + + if (item->command == TODO_COMMENT) { + item->commit = NULL; + item->arg_offset = bol - buf; + item->arg_len = eol - bol; + return 0; + }
And the extra stuff that are only relevant to a comment line is naturally processed by the caller. OK. Thanks. Looking good so far.