Thanks for the review, Eric.
Hopefully, these changes have addressed the concerns that you've raised.
---
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 (5):
t7610: add mergetool --gui tests
mergetool: use get_merge_tool_guessed 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 | 9 +++++-
Documentation/git-mergetool.txt | 4 ++-
builtin/difftool.c | 13 ++++-----
git-mergetool--lib.sh | 39 ++++++++++++++++++--------
git-mergetool.sh | 11 ++------
t/t7610-mergetool.sh | 41 ++++++++++++++++++++++++++++
t/t7800-difftool.sh | 24 ++++++++++++++++
8 files changed, 114 insertions(+), 31 deletions(-)
Range-diff against v1:
1: 678f9b11fc = 1: 678f9b11fc t7610: add mergetool --gui tests
2: 692875cf4b ! 2: e928db892e mergetool: use get_merge_tool function
@@ -1,15 +1,19 @@
Author: Denton Liu <liu.denton@xxxxxxxxx>
- mergetool: use get_merge_tool function
+ mergetool: use get_merge_tool_guessed function
In git-mergetool, the logic for getting which merge tool to use is
duplicated in git-mergetool--lib, except for the fact that it needs to
know whether the tool was guessed or not.
- Rewrite `get_merge_tool` to return whether or not the tool was guessed
- and make git-mergetool call this function instead of duplicating the
- logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not
- the guitool will be selected.
+ Write `get_merge_tool_guessed` to return whether or not the tool was
+ guessed in addition to the actual tool and make git-mergetool call this
+ function instead of duplicating the logic. Also, let
+ `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will
+ be selected.
+
+ Make `get_merge_tool` use this function internally so that code
+ duplication is reduced.
Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
@@ -17,38 +21,32 @@
--- a/Documentation/git-mergetool--lib.txt
+++ b/Documentation/git-mergetool--lib.txt
@@
+
FUNCTIONS
---------
- get_merge_tool::
-- returns a merge tool.
++get_merge_tool_guessed::
+ returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if
+ the tool was guessed, else 'false'. '$merge_tool' is the merge
+ tool to use. '$GIT_MERGETOOL_GUI' may be set to 'true' to search
+ for the appropriate guitool.
++
+ get_merge_tool::
+- returns a merge tool.
++ returns a merge tool. '$GIT_MERGETOOL_GUI' may be set to 'true'
++ to search for the appropriate guitool.
get_merge_tool_cmd::
returns the custom command for a merge tool.
- diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
- --- a/git-difftool--helper.sh
- +++ b/git-difftool--helper.sh
-@@
- then
- merge_tool="$GIT_DIFF_TOOL"
- else
-- merge_tool="$(get_merge_tool)" || exit
-+ merge_tool="$(get_merge_tool | sed -e 's/^[a-z]*://')" || exit
- fi
- fi
-
-
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@
+ echo "$merge_tool_path"
}
- get_merge_tool () {
+-get_merge_tool () {
++get_merge_tool_guessed () {
+ is_guessed=false
# Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
@@ -61,6 +59,10 @@
fi
- echo "$merge_tool"
+ echo "$is_guessed:$merge_tool"
++}
++
++get_merge_tool () {
++ get_merge_tool_guessed | sed -e 's/^[a-z]*://'
}
mergetool_find_win32_cmd () {
@@ -81,7 +83,7 @@
- guessed_merge_tool=true
- fi
+ IFS=':' read guessed_merge_tool merge_tool <<-EOF
-+ $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool)
++ $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool_guessed)
+ EOF
fi
merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
3: de1b897a11 = 3: 24db1afeee mergetool: fallback to tool when guitool unavailable
4: a272594bd2 = 4: 6f65b5c913 difftool: make --gui, --tool and --extcmd mutually exclusive
5: 4fc3f84bad = 5: 5a24772219 difftool: fallback on merge.guitool
--
2.21.0.1000.g11cd861522
