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

[Junio C Hamano <gitster@pobox.com>]

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

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

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

> 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";;

;-)