Re: Feature Request: user defined suffix for temp files created by git-mergetool

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

Josef Ridky <jridky@redhat.com> writes:

> Hi, 
>
> I have just realize, that my attachment has been cut off from my previous message.
> Below you can find patch with requested change.
>
> Add support for user defined suffix part of name of temporary files
> created by git mergetool
> ---

The first two paragraphs above do not look like they are meant for
the commit log for this change.

Please sign-off your patch.

> -USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'
> +USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [--local=name] [--remote=name] [--backup=name] [--base=name] [file to merge] ...'
>  SUBDIRECTORY_OK=Yes
>  NONGIT_OK=Yes
>  OPTIONS_SPEC=
>  TOOL_MODE=merge
> +
> +#optional name space convention
> +local_name=""
> +remote_name=""
> +base_name=""
> +backup_name=""

If you initialize these to LOCAL, REMOTE, etc. instead of empty,
wouldn't it make the remainder of the code a lot simpler?  For
example,

> +	if [ "$local_name" != "" ]  && [ "$local_name" != "$remote_name" ] && [ "$local_name" != "$backup_name" ] && [ "$local_name" != "$base_name" ]
> +	then
> +		LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"
> +	else
> +		LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
> +	fi

This can just be made an unconditional

	LOCAL="$MERGETOOL_TMPDIR/${BASE}_${local_name}_$$$ext"

without any "if" check in front.  The same for all others.

The conditional you added is doing two unrelated things.  It is
trying to switch between an unset $local_name and default LOCAL,
while it tries to make sure the user did not give the same string
for two different things (which is a nonsense).  It is probably
better to check for nonsense just once just before all these
assuments of LOCAL, REMOTE, etc. begins, not at each point where
they are set like this patch does.