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@xxxxxxxxx>
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
--
2.21.0.1000.g11cd861522
