Re: [PATCH v2 20/23] rebase -i: parse to-do list command line options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Fabian Ruch <bafain@xxxxxxxxx> writes:

[...]
> are not supported at the moment. Neither are options that contain
> spaces because the shell expansion of `args` in `do_next` interprets
> white space characters as argument separator, that is a command line
> like
>
>     pick --author "A U Thor" fa1afe1 Some change
>
> is parsed as the pick command
>
>     pick --author
>
> and the commit hash
>
>     "A
>
> which obviously results in an unknown revision error. For the sake of
> completeness, in the example above the message title variable `rest`
> is assigned the string 'U Thor" fa1afe1 Some change' (without the
> single quotes).

You could probably trim down the non-example a bit and instead give an
example :-)

> Print an error message for unknown or unsupported command line
> options, which means an error for all specified options at the
> moment.

Can you add a test that verifies we catch an obvious unknown option
(such as --unknown-option)?

> Cleanly break the `do_next` loop by assigning the special
> value 'unknown' to the local variable `command`, which triggers the
> unknown command case in `do_cmd`.
[...]
>  do_replay () {
>  	command=$1
> -	sha1=$2
> -	rest=$3
> +	shift
> +
> +	opts=
> +	while test $# -gt 0
> +	do
> +		case "$1" in
> +		-*)
> +			warn "Unknown option: $1"
> +			command=unknown
> +			;;
> +		*)
> +			break
> +			;;

This seems a rather hacky solution to me.  Doesn't this now print

  warning: Unknown option: --unknown-option
  warning: Unknown command: pick --unknown-option

?

It shouldn't claim the command is unknown if the command itself was
valid.

Also, you speak of do_cmd above, but the unknown command handling seems
to be part of do_replay?

-- 
Thomas Rast
tr@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]