Hi Junio,
thank you very much for the tips. I'll make new patch with a simpler code.
Josef
| Sent: Wednesday, October 5, 2016 6:05:59 PM
| Josef Ridky <jridky@xxxxxxxxxx> 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.
|