Re: [PATCH 2/2] Feature Request: user defined suffix for temp files created by git-mergetool

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2016, 17:06 UTC
Message-ID: <xmqq7f9lmmol.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <255814448.2197583.1475759366093.JavaMail.zimbra@redhat.com>

Josef Ridky <jridky@redhat.com> writes:

Show 6 quoted lines

> +	--local=*)
> +		temp_name=${1#--local=}
> +		if [ "$temp_name" != "" ] && [ "$temp_name" != "$REMOTE_NAME" ] && [ "$temp_name" != "$BASE_NAME" ] && [ "$temp_name" != "$BACKUP_NAME" ]
> +		then
> +			LOCAL_NAME=$temp_name
> +		fi

It is not a good idea to ignore an explicit user request without giving any indication and without giving any explanation.

You may have noticed that we do not say "[ cond ]" in this shell script (we say "test" instead; see Documentation/CodingGuidelines).

Instead of having such a test all over the place, I'd suggest doing it outside the loop:

    while test $# != 0
    do
	case "$1" in
	...
	--local=*)
		LOCAL_NAME=${1#--local=} ;;
	--remote=*)                
		REMOTE_NAME=${1#--local=} ;;
	...
        esac
    done
    
    # sanity check _after_ parsing the command line
    case ",$LOCAL_NAME,$REMOTE_NAME,$BASE_NAME,$BACKUP_NAME," in
    *,,*) die "You cannot set --local/remote/... to empty" ;;
    esac
    ... similarly, duplicate check comes here ...