On 21/02/2023 14:24, Derrick Stolee wrote:
On 2/20/2023 9:19 AM, Phillip Wood via GitGitGadget wrote:
From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
+static int check_label_or_ref_arg(enum todo_command command, const char *arg)
+{
+ switch (command) {
+ case TODO_LABEL:
+ /*
+ * '#' is not a valid label as the merge command uses it to
+ * separate merge parents from the commit subject.
+ */
+ if (!strcmp(arg, "#") ||
+ check_refname_format(arg, REFNAME_ALLOW_ONELEVEL))
Tabbing is strange here. Within the case there seems to be "\t " to
the left of each line. Then the conditional has strange spacing. I
think it should be:
if (!strcmp(arg, "#") ||
check_refname_format(arg, REFNAME_ALLOW_ONELEVEL))
(The "check_refname_format()" line is correct in your patch, but the
lines above it are not, for some reason.)
Yes you're right, I'm not sure what happened there. I'll re-roll
Thanks
Phillip
The rest of the switch statement is correctly tabbed.
@@ -2525,10 +2553,19 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
if (item->command == TODO_EXEC || item->command == TODO_LABEL ||
item->command == TODO_RESET || item->command == TODO_UPDATE_REF) {
+ int ret = 0;
+
item->commit = NULL;
item->arg_offset = bol - buf;
item->arg_len = (int)(eol - bol);
- return 0;
+ if (item->command == TODO_LABEL ||
+ item->command == TODO_UPDATE_REF) {
+ saved = *eol;
+ *eol = '\0';
+ ret = check_label_or_ref_arg(item->command, bol);
+ *eol = saved;
+ }
+ return ret;
}
This diff is much simpler to understand for the purpose of this
patch. I saw your comment about splitting out TODO_EXEC for a
future change, and that would be fine when it happens, too.
Thanks for the updates. Outside of that strange whitespace issue,
this patch LGTM.
Thanks,
-Stolee
