patchgit mergetool: Don't repeat merge tool candidates

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(-)

Johannes Gilger <heipei@hackvalue.de> writes:

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.

git mergetool listed some candidates for mergetools twice, depending on the environment.

Signed-off-by: Johannes Gilger <heipei@hackvalue.de>
---
The first patch had the fatal flaw that it listed nothing when DISPLAY 
and EDITOR/VISUAL were unset, we fixed that.
The order in which merge-candidates appear is still exactly the same, 
only duplicates have been stripped. The check for KDE_FULL_SESSION was 
removed since kdiff3 was added as long as DISPLAY was set and we weren't 
running gnome.

 git-mergetool.sh |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Johannes Gilger <heipei@hackvalue.de> writes:

The old code produced this:

   DISPLAY set
   | GNOME_DESKTOP_SESSION_ID set
   | | KDE_FULL_SESSION true
   | | |
   - - - (editor)
   - - + (editor)
   - + - (editor)
   - + + (editor)
   + - - kdiff3 tkdiff xxdiff meld gvimdiff (editor)
   + - + kdiff3 kdiff3 tkdiff xxdiff meld gvimdiff (editor)
   + + - meld kdiff3 tkdiff xxdiff meld gvimdiff (editor)
   + + + kdiff3 meld kdiff3 tkdiff xxdiff meld gvimdiff (editor)

where (editor) lists emerge or vimdiff if the preferred editor was emacs or vim, and then opendiff, and then emerge and vimdiff as fallback duplicates.

Looking at what the new code does for the (editor) fallback part first:

   if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
       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 opendiff emerge"
   fi
   if test -z "$merge_tool_candidates" ; then
       merge_tool_candidates="opendiff emerge vimdiff"
   fi

I think it is better to rewrite this part for clarity:

    if EDITOR is emacs?
    then
    	append emerge opendiff vimdiff in this order
    elif EDITOR is vim?
    then
    	append vimdiff opendiff emerge in this order
    else
    	append opendiff emerge vimdiff in this order
    fi

because emacs and vim cannot be set to EDITOR at the same time (note that I think this also fixes a bug; see below).

This one produces:

   DISPLAY set
   | GNOME_DESKTOP_SESSION_ID set
   | | KDE_FULL_SESSION true
   | | |
   - - - (editor)
   - - + (editor)
   - + - (editor)
   - + + (editor)
   + - - kdiff3 tkdiff xxdiff meld gvimdiff (editor')
   + - + kdiff3 tkdiff xxdiff meld gvimdiff (editor')
   + + - meld kdiff3 tkdiff xxdiff gvimdiff (editor')
   + + + meld kdiff3 tkdiff xxdiff gvimdiff (editor')

where "(editor')" is empty if your EDITOR is not emacs nor vim.

The original list with the duplicates removed is:

   DISPLAY set
   | GNOME_DESKTOP_SESSION_ID set
   | | KDE_FULL_SESSION true
   | | |
   - - - (editor)
   - - + (editor)
   - + - (editor)
   - + + (editor)
   + - - kdiff3 tkdiff xxdiff meld gvimdiff (editor)
   + - + kdiff3 tkdiff xxdiff meld gvimdiff (editor)
   + + - meld kdiff3 tkdiff xxdiff gvimdiff (editor)
   + + + kdiff3 meld tkdiff xxdiff gvimdiff (editor)

Aside from the "(editor') is empty when DISPLAY is set" difference, the result is also different iff GNOME_DESKTOP_SESSION_ID and KDE_FULL_SESSION are both set. I am guessing that that does not happen in a sane environment, though.

git mergetool listed some candidates for mergetools twice, depending on the environment.

Signed-off-by: Johannes Gilger <heipei@hackvalue.de>
---
This improves on v2 of this patch as it appends non-gui merge-tools even 
if $DISPLAY is set. It still makes the assumption that KDE_FULL_SESSION 
and GNOME_DESKTOP_SESSION_ID are never set at the same time. If this 
were to happen the tool would simply prefer meld over kdiff3.

 git-mergetool.sh |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)