Re: [PATCH v2] mergetools/diffmerge: support DiffMerge as a git mergetool

From: David Aguilar <davvid@gmail.com>
Date: Oct 12, 2013, 19:55 UTC
Message-ID: <CAJDDKr7LgCXjZLzZqKkkUnqt4EGdcA0wvf=uT8KZXba-+q00Sw@mail.gmail.com>
In-Reply-To: <1381554070-12241-1-git-send-email-ssaasen@atlassian.com>

Thanks for the re-roll.  We're very close; see below.

On Fri, Oct 11, 2013 at 10:01 PM, Stefan Saasen <ssaasen@atlassian.com> wrote:

Show 43 quoted lines

> DiffMerge is a non-free (but gratis) tool that supports OS X, Windows and Linux.
>
>     See http://www.sourcegear.com/diffmerge/
>
> DiffMerge includes a script `/usr/bin/diffmerge` that can be used to launch the
> graphical compare tool.
>
> This change adds mergetool support for DiffMerge and adds 'diffmerge' as an
> option to the mergetool help.
>
> Signed-off-by: Stefan Saasen <ssaasen@atlassian.com>
> Acked-by: David Aguilar <davvid@gmail.com>
> ---
>  contrib/completion/git-completion.bash |  2 +-
>  git-mergetool--lib.sh                  |  3 ++-
>  mergetools/diffmerge                   | 15 +++++++++++++++
>  3 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 mergetools/diffmerge
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index e1b7313..07b0ba5 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1188,7 +1188,7 @@ _git_diff ()
>         __git_complete_revlist_file
>  }
>
> -__git_mergetools_common="diffuse ecmerge emerge kdiff3 meld opendiff
> +__git_mergetools_common="diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
>                         tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare
>  "
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index feee6a4..0fcb253 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -250,7 +250,8 @@ list_merge_tool_candidates () {
>                 else
>                         tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
>                 fi
> -               tools="$tools gvimdiff diffuse ecmerge p4merge araxis bc3 codecompare"
> +               tools="$tools gvimdiff diffuse diffmerge ecmerge "
> +               tools+="p4merge araxis bc3 codecompare"

I don't believe "+=" is portable across all POSIX shells.

I tried this on "dash" (which is the default /bin/sh on Debian) and it was not understood there.

$ f="1 2 3" $ f+=" 4" /bin/dash: 2: f+= 4: not found

I think we should stick to the tools="$tools ....." style of concatenation.

Everything else looks good to me.

Thanks,

-- 
David