Josef Ridky <jri...@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.
