Re: [PATCH v3 8/9] rebase: add the --gpg-sign option

> On Mon, Feb 03, 2014 at 01:42:06PM -0800, Junio C Hamano wrote:
>>> +	--gpg-sign)
>>> +		gpg_sign_opt=-S
>>> +		;;
>>> +	--gpg-sign=*)
>>> +		# Try to quote only the argument, as this will appear in human-readable
>>> +		# output as well as being passed to commands.
>>> +		gpg_sign_opt="-S$(git rev-parse --sq-quote "${1#--gpg-sign=}" |
>>> +			sed 's/^ //')"
>>
>> Isn't an invocation of sed excessive?
>>
>> 	gpg_sign_opt=$(git rev-parse --sq-quote "${1#--gpg-sign=}") &&
>> 	gpg_sign_opt=-S${gpg_sign_opt# }
>>
>> if you really need to strip the leading SP, which I do not think is
>> a necessary thing to do.  It is sufficient to remove the SP before
>> the variable substitution in the human-readable messages, e.g.
>
> I'm not sure that command line parsing of "-S 'foo <x@example.tld>'"
> will work exactly as expected due to the fact that -S doesn't always
> take an argument.  Your suggestion to use # seems fine, though.
>
> I'm a little embarrassed to admit that in my fifteen years of Unix
> experience, I've never learned the variable modifiers for shell, so it
> didn't occur to me to use them in this case.  Guess it's time to learn
> them now.