From: Sebastian Schuberth <sschuberth@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Olivier Croquette <ocroquette@free.fr>
Cc: git@vger.kernel.org, David Aguilar <davvid@gmail.com>
Subject: Re: [PATCH] Copy mergetool "bc3" as "bc4"
Date: Mon, 20 Oct 2014 20:23:48 +0200 [thread overview]
Message-ID: <54455334.8000503@gmail.com> (raw)
In-Reply-To: <xmqqtx2y3avx.fsf@gitster.dls.corp.google.com>
On 20.10.2014 19:32, Junio C Hamano wrote:
>>> Beyond compare 4 is out since september 2014. The CLI interface
>>> doesn't seem to have changed compared to the version 3.
>>
>> Hmph, if this is identical to mergetools/bc3, why is the patch even
>> needed? Do we auto-detect something and try to use bc4 which does
>> not exist and fail, and we must supply a copy as bc4 to prevent it?
The patch is indeed not needed, which is why I haven't cared to provide it so far although I'm now using Beyond Compare 4 instead of version 3 myself.
>> It may feel somewhat strange to have to say "mergetool --tool=bc3"
>> when you know what you have is version 4 and not version 3, but in
That's exactly the only reason I could think of why it could be nice to have a "bc4".
>> that case, I wonder if there are reasons why calling both versions
>> just "bc" is a bad idea. And assuming that version 5 and later
IMHO, the only reason not to just have a single "bc" is to maintain backward compatibility for users already using "bc3". But for the sake of cleaner code, personally I'd be fine with that minor backward compatibility breakage.
>> Perhaps version 2 and before are unusable as a mergetool backend or
>> something?
>
> It seems that ffe6dc08 (mergetool--lib: Add Beyond Compare 3 as a
> tool, 2011-02-27) is the first mention of "Beyond Compare" and it
> only was interested in version 3 and nothing else.
Beyond Compare versions prior to 3 do not run on Linux, but only on Windows, which is why I did not care to submit a patch.
> Perhaps something like this, so that existing users can still use
> "bc3" and other people can use "bc" if it bothers them that they
> have to say "3" when the backend driver works with both 3 and 4?
That indeed sounds like the best approach.
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -250,7 +250,7 @@ list_merge_tool_candidates () {
> tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
> fi
> tools="$tools gvimdiff diffuse diffmerge ecmerge"
> - tools="$tools p4merge araxis bc3 codecompare"
> + tools="$tools p4merge araxis bc bc3 codecompare"
Why keep bc3 here?
And shouldn't we update git-gui/lib/mergetool.tcl, too?
--
Sebastian Schuberth
next prev parent reply other threads:[~2014-10-20 18:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-20 11:03 [PATCH] Copy mergetool "bc3" as "bc4" Olivier Croquette
2014-10-20 16:40 ` Junio C Hamano
2014-10-20 17:32 ` Junio C Hamano
2014-10-20 18:23 ` Sebastian Schuberth [this message]
2014-10-20 18:40 ` Junio C Hamano
2014-10-21 8:44 ` David Aguilar
2014-10-21 18:27 ` Junio C Hamano
2014-10-20 20:35 ` Olivier Croquette
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54455334.8000503@gmail.com \
--to=sschuberth@gmail.com \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ocroquette@free.fr \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.