Re: [PATCH] Copy mergetool "bc3" as "bc4"

From: David Aguilar <davvid@gmail.com>
Date: Oct 21, 2014, 08:44 UTC
Message-ID: <20141021084436.GA39148@gmail.com>
In-Reply-To: <xmqqfvei37qg.fsf@gitster.dls.corp.google.com>

On Mon, Oct 20, 2014 at 11:40:23AM -0700, Junio C Hamano wrote:

Show 23 quoted lines

> Sebastian Schuberth <sschuberth@gmail.com> writes:
> 
> >> 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?
> 
> I didn't carefully look at the code that uses this list to see if we
> have to list everything or can list just the ones we recommend, and
> erred on the safer side (unlike the one for completion where I
> omitted bc3 as "deprecated").

We should drop "bc3" here. This is the list of candidates that is used when no configuration exists. Listing "bc" twice here implies that we might try that candidate twice.

Otherwise, this patch looks like the right way to go ~ it makes "bc" available and keeps compatibility for "bc3".

If we wanted to phase out "bc3" for Git 3.0 we could start warning inside of the "bc3" scriptlet, but I don't see any reason to do so a.t.m.

Thanks,

-- 
David