From: Junio C Hamano <gitster@pobox.com>
To: Josef Ridky <jridky@redhat.com>
Cc: git@vger.kernel.org
Subject: Re: Feature Request: user defined suffix for temp files created by git-mergetool
Date: Wed, 05 Oct 2016 09:05:59 -0700 [thread overview]
Message-ID: <xmqqponerdaw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1499287628.1324571.1475653631366.JavaMail.zimbra@redhat.com> (Josef Ridky's message of "Wed, 5 Oct 2016 03:47:11 -0400 (EDT)")
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.
next prev parent reply other threads:[~2016-10-05 16:06 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <88486231.114620.1475474318974.JavaMail.zimbra@redhat.com>
2016-10-03 6:36 ` Feature Request: user defined suffix for temp files created by git-mergetool Josef Ridky
2016-10-03 15:18 ` Anatoly Borodin
2016-10-04 5:18 ` Josef Ridky
2016-10-05 9:47 ` David Aguilar
2016-10-05 10:19 ` Josef Ridky
2016-10-05 7:47 ` Josef Ridky
2016-10-05 16:05 ` Junio C Hamano [this message]
2016-10-06 6:47 ` Josef Ridky
2016-10-05 21:04 ` Johannes Sixt
2016-10-06 7:21 ` Josef Ridky
2016-10-06 12:43 ` [PATCH 1/2] " Josef Ridky
2016-10-06 13:09 ` [PATCH 2/2] " Josef Ridky
2016-10-06 17:06 ` Junio C Hamano
2016-10-12 8:24 ` [PATCH v2 " Josef Ridky
2016-10-12 17:59 ` Junio C Hamano
2016-10-13 4:55 ` David Aguilar
2016-10-13 5:13 ` [PATCH 1/2] " David Aguilar
2016-10-06 16:58 ` Junio C Hamano
2016-10-06 15:54 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqponerdaw.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jridky@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).