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.)
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
