Re: [PATCH 1/2] rebase -i: match whole word in is_command()

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2023, 22:25 UTC
Message-ID: <xmqq5ybs6mv0.fsf@gitster.g>
In-Reply-To: <b050e7c1a4daf6c65aa9b2b6f44248d7b403918e.1677185702.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

>  	const char *str = todo_command_info[command].str;
>  	const char nick = todo_command_info[command].c;
> -	const char *p = *bol + 1;
> +	const char *p = *bol;
>  
> -	return skip_prefix(*bol, str, bol) ||
> -		((nick && **bol == nick) &&
> -		 (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&
> -		 (*bol = p));
> +	return (skip_prefix(p, str, &p) || (nick && *p++ == nick)) &&
> +		(*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&
> +		(*bol = p);

OK. So we skip the command name string in the line given by the end-user (or see if the first letter matches the single letter command) and make sure it is followed by a whitespace or EOL in either case. The old code was not doing the "end of word" check for the longhand at all, which was clearly wrong.

I too find "&& (*bol = p)" that pretends to be a Boolean condition but is there only for its side effect distasteful, but I agree with you that fixing it is outside the scope of this patch.

Show 7 quoted lines

> @@ -2513,7 +2512,8 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
>  			break;
>  		}
>  	if (i >= TODO_COMMENT)
> -		return -1;
> +		return error(_("invalid command '%.*s'"),
> +			     (int)strcspn(bol, " \t\r\n"), bol);

Nice.

Show 10 quoted lines

> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index b57541356bd..1d2f0429aea 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -60,7 +60,7 @@ set_fake_editor () {
>  		">")
>  			echo >> "$1";;
>  		bad)
> -			action="badcmd";;
> +			action="pickled";;

;-)