[PATCH v4 0/6] difftool and mergetool improvements

Thanks for another review, Junio.

This should basically be the same as v3 except I've moved the IFS assignment into a subshell so its value does not leak out.

I'd like the change in 4/6 to be reviewed carefully. t7610 and t7800 run properly with dash on My Machineة, but I couldn't find a reference on whether my usage is POSIX legal or not.

We do have precedent for using fors within a subshell, though, from a quick git grep:

	t/t3202-show-branch-octopus.sh: git show-branch $(for i in $numbers; do echo branch$i; done) > out &&

Thanks,

Denton

---

Changes since v3:

* Move nested for into a subshell so that IFS does not leak out of
  function

Changes since v2:

* Unsuppress output in t7610
* Make `get_merge_tool` return 1 on guessed so we don't have to deal
  with parsing '$guessed:$merge_tool'

Changes since v1:

* Introduce get_merge_tool_guessed function instead of changing
  get_merge_tool
* Remove unnecessary if-tower in mutual exclusivity logic
Denton Liu (6):
  t7610: unsuppress output
  t7610: add mergetool --gui tests
  mergetool: use get_merge_tool function
  mergetool: fallback to tool when guitool unavailable
  difftool: make --gui, --tool and --extcmd mutually exclusive
  difftool: fallback on merge.guitool

 Documentation/git-difftool.txt       |   4 +-
 Documentation/git-mergetool--lib.txt |   4 +-
 Documentation/git-mergetool.txt      |   4 +-
 builtin/difftool.c                   |  13 +--
 git-difftool--helper.sh              |   2 +-
 git-mergetool--lib.sh                |  37 ++++--
 git-mergetool.sh                     |  12 +-
 t/t7610-mergetool.sh                 | 163 +++++++++++++++++----------
 t/t7800-difftool.sh                  |  24 ++++
 9 files changed, 172 insertions(+), 91 deletions(-)

Range-diff against v3:
1:  9f9922cab3 = 1:  9f9922cab3 t7610: unsuppress output
2:  0f632ca6bf = 2:  0f632ca6bf t7610: add mergetool --gui tests
3:  ff83d25ff2 = 3:  ff83d25ff2 mergetool: use get_merge_tool function
4:  e975fe4a8b ! 4:  c799e871e2 mergetool: fallback to tool when guitool unavailable
    @@ -6,14 +6,18 @@
         not set, it falls back to `diff.tool`. Make git-mergetool also fallback
         from `merge.guitool` to `merge.tool` if the former is undefined.
     
    -    If git-difftool were to use `get_configured_mergetool`, it would also
    -    get the fallback behaviour in the following precedence:
    +    If git-difftool, when called with `--gui`, were to use
    +    `get_configured_mergetool` in a future patch, it would also get the
    +    fallback behavior in the following precedence:
     
         1. diff.guitool
         2. merge.guitool
         3. diff.tool
         4. merge.tool
     
    +    Note that the behavior for when difftool or mergetool are called without
    +    `--gui` should be identical with or without this patch.
    +
         Signed-off-by: Denton Liu <liu.denton@gmail.com>
     
      diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
    @@ -61,14 +65,19 @@
     +		keys="guitool $keys"
      	fi
     +
    -+	IFS=' '
    -+	for key in $keys
    -+	do
    -+		for section in $sections
    ++	merge_tool=$(
    ++		IFS=' '
    ++		for key in $keys
     +		do
    -+			merge_tool=$(git config $section.$key) && break 2
    -+		done
    -+	done
    ++			for section in $sections
    ++			do
    ++				if selected=$(git config $section.$key)
    ++				then
    ++					echo "$selected"
    ++					return
    ++				fi
    ++			done
    ++		done)
     +
      	if test -n "$merge_tool" && ! valid_tool "$merge_tool"
      	then
5:  bc3e229171 = 5:  7d7c936cd3 difftool: make --gui, --tool and --extcmd mutually exclusive
6:  f39b15efbd = 6:  b642ed3b1e difftool: fallback on merge.guitool