Re: [PATCH] git mergetool: Don't repeat merge tool candidates

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 23, 2009, 08:16 UTC
Message-ID: <7v7i4m1lq4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1232569442-12480-1-git-send-email-heipei@hackvalue.de>

Johannes Gilger <heipei@hackvalue.de> writes:

Show 39 quoted lines

> git mergetool listed some candidates for mergetools twice, depending on
> the environment.
>
> Signed-off-by: Johannes Gilger <heipei@hackvalue.de>
> ---
>  git-mergetool.sh |   13 +++++--------
>  1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 00e1337..8f09e4a 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -390,21 +390,18 @@ fi
>  
>  if test -z "$merge_tool" ; then
>      if test -n "$DISPLAY"; then
> -        merge_tool_candidates="kdiff3 tkdiff xxdiff meld gvimdiff"
>          if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> -            merge_tool_candidates="meld $merge_tool_candidates"
> -        fi
> -        if test "$KDE_FULL_SESSION" = "true"; then
> -            merge_tool_candidates="kdiff3 $merge_tool_candidates"
> +            merge_tool_candidates="meld kdiff3 tkdiff xxdiff gvimdiff"
> +        else
> +            merge_tool_candidates="kdiff3 tkdiff xxdiff meld gvimdiff"
>          fi
>      fi
>      if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
> -        merge_tool_candidates="$merge_tool_candidates emerge"
> +        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
>      fi
>      if echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
> -        merge_tool_candidates="$merge_tool_candidates vimdiff"
> +        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
>      fi
> -    merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
>      echo "merge tool candidates: $merge_tool_candidates"
>      for i in $merge_tool_candidates; do
>          init_merge_tool_path $i

Doesn't this change the order of the tools listed in the variable, affecting which one ends up being used? I think that is a worse regression than repeating the same name twice in an otherwise no-op informational message.

Please spend a few minutes to see if there are active developers who are familiar with the area of code you are touching and Cc them to ask their input.

    git blame -L390,+20 git-mergetool.sh

tells me that most of this came from 301ac38 (git-mergetool: Make default selection of merge-tool more intelligent, 2007-06-10), so I am Cc'ing Ted.