From 05fb8726cccc74908853c166248ff9b6abdafae5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:08 -0400 Subject: mergetool: use get_merge_tool 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 through the return code and make git-mergetool call this function instead of duplicating the logic. Note that 1 was chosen to be the return code of when a tool is guessed because it seems like a slightly more abnormal condition than getting a tool that's explicitly specified but this is completely arbitrary. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will be selected. This change is not completely backwards compatible as there may be external users of git-mergetool--lib. However, only one user, git-diffall[1], was found from searching GitHub and Google, and this tool is superseded by `git difftool --dir-diff` anyway. It seems very unlikely that there exists an external caller that would take into account the return code of `get_merge_tool` as it would always return 0 before this change so this change probably does not affect any external users. [1]: https://github.com/thenigan/git-diffall Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'git-mergetool--lib.sh') diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 83bf52494c..b928179a2e 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,17 @@ get_merge_tool_path () { } get_merge_tool () { + is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool) + merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then merge_tool=$(guess_merge_tool) || exit + is_guessed=true fi echo "$merge_tool" + test "$is_guessed" = false } mergetool_find_win32_cmd () { -- cgit v1.2.3 From 884630b2e2634969656faabc7f4e33cab2e32b35 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:11 -0400 Subject: mergetool--lib: create gui_mode function Before, in `get_configured_merge_tool`, we would test the value of the first argument directly, which corresponded to whether we were using guitool. However, since `$GIT_MERGETOOL_GUI` is available as an environment variable, create the `gui_mode` function which increases the clarify of functions which use it. While we're at it, add a space before `()` in function definitions to fix the style. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'git-mergetool--lib.sh') diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index b928179a2e..4ca170c8a7 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -80,14 +80,18 @@ show_tool_names () { } } -diff_mode() { +diff_mode () { test "$TOOL_MODE" = diff } -merge_mode() { +merge_mode () { test "$TOOL_MODE" = merge } +gui_mode () { + test "$GIT_MERGETOOL_GUI" = true +} + translate_merge_tool_path () { echo "$1" } @@ -350,8 +354,7 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + if gui_mode then gui_prefix=gui fi @@ -405,7 +408,7 @@ get_merge_tool_path () { get_merge_tool () { is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) + merge_tool=$(get_configured_merge_tool) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then -- cgit v1.2.3 From 60aced3dfa68df60952fed28c4ae63a5bbda0275 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:14 -0400 Subject: mergetool: fallback to tool when guitool unavailable In git-difftool, if the tool is called with --gui but `diff.guitool` is 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, 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 The behavior for when difftool or mergetool are called without `--gui` should be identical with or without this patch. Note that the search loop could be written as sections="merge" keys="tool" if diff_mode then sections="diff $sections" fi if gui_mode then keys="guitool $keys" fi merge_tool=$( IFS=' ' for key in $keys do for section in $sections do selected=$(git config $section.$key) if test -n "$selected" then echo "$selected" return fi done done) which would make adding a mode in the future much easier. However, adding a new mode will likely never happen as it is highly discouraged so, as a result, it is written in its current form so that it is more readable for future readers. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) (limited to 'git-mergetool--lib.sh') diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 4ca170c8a7..696eb49160 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -354,19 +354,36 @@ guess_merge_tool () { } get_configured_merge_tool () { - if gui_mode - then - gui_prefix=gui - fi - - # Diff mode first tries diff.(gui)tool and falls back to merge.(gui)tool. - # Merge mode only checks merge.(gui)tool + keys= if diff_mode then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) + if gui_mode + then + keys="diff.guitool merge.guitool diff.tool merge.tool" + else + keys="diff.tool merge.tool" + fi else - merge_tool=$(git config merge.${gui_prefix}tool) + if gui_mode + then + keys="merge.guitool merge.tool" + else + keys="merge.tool" + fi fi + + merge_tool=$( + IFS=' ' + for key in $keys + do + selected=$(git config $key) + if test -n "$selected" + then + echo "$selected" + return + fi + done) + if test -n "$merge_tool" && ! valid_tool "$merge_tool" then echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to unknown tool: $merge_tool" -- cgit v1.2.3