Re: [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr`

[Johannes Sixt <j6t@kdbg.org>]

Am 10.06.19 um 19:17 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>> git-mergetool spawns an enormous amount of processes. For this reason,
>> the test script, t7610, is exceptionally slow, in particular, on
>> Windows. Most of the processes are invocations of git, but there are
>> also some that can be replaced with shell builtins. Do so with `expr`.
> 
> I see these as improvements independent of whatever test may or may
> not be slow ;-)  s/^.*/but there are/There are/.  Thanks for working
> on it.

Noted.

>> @@ -255,13 +254,16 @@ merge_file () {
>>  		return 1
>>  	fi
>>  
>> -	if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$')
>> -	then
>> -		ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$')
>> -	else
>> +	# extract file extension from the last path component
>> +	case "${MERGED##*/}" in
>> +	*.*)
>> +		ext=.${MERGED##*.}
>> +		BASE=${MERGED%"$ext"}
> 
> This rewrite can potentially change the behaviour, when $ext has
> glob metacharacters.  Wouldn't BASE=${MERGED%.*} be more faithful
> conversion?

Since "$ext" is quoted inside the braces of the parameter expansion, the
pattern counts as quoted, so all glob characters in $ext lose their
special meaning. At least that's how I read the spec.

I do see the symmetry in your proposed version. Nevertheless, I have a
slight preference for my version because it specifies exactly what is to
be removed from the end of value.

-- Hannes