From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
Refactor the mergetool-lib so that we can reuse it in
Documentation/Makefile. The end result is that the
diff.tool and merge.tool documentation now includes
an auto-generated list of all available tools.
This applies on top of jk/mergetool in pu.
David Aguilar (4):
mergetool--lib: Simplify command expressions
mergetool--lib: Improve the help text in guess_merge_tool()
mergetool--lib: Add functions for finding available tools
doc: Generate a list of valid merge tools
Documentation/.gitignore | 1 +
Documentation/Makefile | 16 +++++-
Documentation/diff-config.txt | 13 ++---
Documentation/merge-config.txt | 12 ++---
git-mergetool--lib.sh | 108 ++++++++++++++++++++++-------------------
5 files changed, 87 insertions(+), 63 deletions(-)
--
1.8.0.13.gf25ae33
From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
Use $(command "$arg") instead of "$(command "$arg")" as the latter is
harder to read. Make the expression in get_merge_tool_cmd() even
simpler by avoiding "echo" completely.
Signed-off-by: David Aguilar <redacted>
---
git-mergetool--lib.sh | 40 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 24 deletions(-)
@@ -96,14 +89,13 @@ setup_tool () {} get_merge_tool_cmd(){-# Prints the custom command for a merge toolmerge_tool="$1"ifdiff_modethen-echo"$(gitconfigdifftool.$merge_tool.cmd||-gitconfigmergetool.$merge_tool.cmd)"+gitconfig"difftool.$merge_tool.cmd"||+gitconfig"mergetool.$merge_tool.cmd"else-echo"$(gitconfigmergetool.$merge_tool.cmd)"+gitconfig"mergetool.$merge_tool.cmd"fi}
@@ -145,7 +137,7 @@ run_merge_tool () {# Run a either a configured or built-in diff tool run_diff_cmd(){-merge_tool_cmd="$(get_merge_tool_cmd"$1")"+merge_tool_cmd=$(get_merge_tool_cmd"$1")iftest-n"$merge_tool_cmd"then(eval$merge_tool_cmd)
@@ -158,11 +150,11 @@ run_diff_cmd () {# Run a either a configured or built-in merge tool run_merge_cmd(){-merge_tool_cmd="$(get_merge_tool_cmd"$1")"+merge_tool_cmd=$(get_merge_tool_cmd"$1")iftest-n"$merge_tool_cmd"then-trust_exit_code="$(gitconfig--bool\-mergetool."$1".trustExitCode||echofalse)"+trust_exit_code=$(gitconfig--bool\+"mergetool.$1.trustExitCode"||echofalse)iftest"$trust_exit_code"="false"thentouch"$BACKUP"
@@ -253,7 +245,7 @@ guess_merge_tool () {# Loop over each candidate and stop when a valid merge tool is found.foriin$toolsdo-merge_tool_path="$(translate_merge_tool_path"$i")"+merge_tool_path=$(translate_merge_tool_path"$i")iftype"$merge_tool_path">/dev/null2>&1thenecho"$i"
@@ -300,9 +292,9 @@ get_merge_tool_path () {fiiftest-z"$merge_tool_path"then-merge_tool_path="$(translate_merge_tool_path"$merge_tool")"+merge_tool_path=$(translate_merge_tool_path"$merge_tool")fi-iftest-z"$(get_merge_tool_cmd"$merge_tool")"&&+iftest-z$(get_merge_tool_cmd"$merge_tool")&&!type"$merge_tool_path">/dev/null2>&1thenecho>&2"The $TOOL_MODE tool $merge_tool is not available as"\
@@ -314,11 +306,11 @@ get_merge_tool_path () { get_merge_tool(){# Check if a merge tool has been configured-merge_tool="$(get_configured_merge_tool)"+merge_tool=$(get_configured_merge_tool)# Try to guess an appropriate merge tool if no tool has been set.iftest-z"$merge_tool"then-merge_tool="$(guess_merge_tool)"||exit+merge_tool=$(guess_merge_tool)||exitfiecho"$merge_tool"}
From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
This code path is only activated when the user does not have a valid
configured tool. Add a message to guide new users towards configuring a
default tool.
Signed-off-by: David Aguilar <redacted>
---
git-mergetool--lib.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
@@ -240,7 +240,14 @@ show_tool_help () { guess_merge_tool(){list_merge_tool_candidates-echo>&2"merge tool candidates: $tools"+msg="\++Thismessageisdisplayedbecause'$TOOL_MODE.tool'isnotconfigured.+See'git ${TOOL_MODE}tool --tool-help'or'git help config'formoredetails.+'git ${TOOL_MODE}tool'willnowattempttouseoneofthefollowingtools:+$tools+"+printf"$msg">&2# Loop over each candidate and stop when a valid merge tool is found.foriin$tools
From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
Use the new filter_tools() function to build lists of all
the built-in tools supported by difftool and mergetool.
This frees us from needing to update the documentation
whenever a new tool is added.
Signed-off-by: David Aguilar <redacted>
---
Documentation/.gitignore | 1 +
Documentation/Makefile | 16 +++++++++++++++-
Documentation/diff-config.txt | 13 +++++++------
Documentation/merge-config.txt | 12 ++++++------
git-mergetool--lib.sh | 1 +
5 files changed, 30 insertions(+), 13 deletions(-)
@@ -132,9 +132,10 @@ diff.<driver>.cachetextconv:: conversion outputs. See linkgit:gitattributes[5] for details. diff.tool::- The diff tool to be used by linkgit:git-difftool[1]. This- option overrides `merge.tool`, and has the same valid built-in- values as `merge.tool` minus "tortoisemerge" and plus- "kompare". Any other value is treated as a custom diff tool,- and there must be a corresponding `difftool.<tool>.cmd`- option.+ Controls which diff tool is used by linkgit:git-difftool[1].+ This variable overrides the value configured in `merge.tool`.+ The list below shows the valid built-in values.+ Any other value is treated as a custom diff tool and requires+ that a corresponding difftool.<tool>.cmd variable is defined.++include::mergetools-diff.txt[]
@@ -52,12 +52,12 @@ merge.stat:: at the end of the merge. True by default. merge.tool::- Controls which merge resolution program is used by- linkgit:git-mergetool[1]. Valid built-in values are: "araxis",- "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld",- "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff"- and "xxdiff". Any other value is treated is custom merge tool- and there must be a corresponding mergetool.<tool>.cmd option.+ Controls which merge tool is used by linkgit:git-mergetool[1].+ The list below shows the valid built-in values.+ Any other value is treated as a custom merge tool and requires+ that a corresponding mergetool.<tool>.cmd variable is defined.++include::mergetools-merge.txt[] merge.verbosity:: Controls the amount of output shown by the recursive merge
@@ -1,5 +1,6 @@#!/bin/sh# git-mergetool--lib is a library for common merge tool functions+test-z"$MERGE_TOOLS_DIR"&&MERGE_TOOLS_DIR=$(git--exec-path)/mergetools mode_ok(){
From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
Refactor show_tool_help() so that the tool-finding logic is broken out
into separate functions.
Signed-off-by: David Aguilar <redacted>
---
git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 26 deletions(-)
@@ -2,6 +2,33 @@# git-mergetool--lib is a library for common merge tool functionsMERGE_TOOLS_DIR=$(git--exec-path)/mergetools+mode_ok(){+diff_mode&&can_diff||+merge_mode&&can_merge+}++is_available(){+merge_tool_path=$(translate_merge_tool_path"$1")&&+type"$merge_tool_path">/dev/null2>&1+}++filter_tools(){+filter="$1"+prefix="$2"+(+cd"$MERGE_TOOLS_DIR"&&+foriin*+do+echo"$i"+done+)|sort|whilereadtool+do+setup_tool"$tool"2>/dev/null&&+(eval"$filter""$tool")&&+printf"$prefix$tool\n"+done+}+ diff_mode(){test"$TOOL_MODE"=diff}
@@ -199,27 +226,13 @@ list_merge_tool_candidates () {} show_tool_help(){-unavailable=available=LF='-'-foriin"$MERGE_TOOLS_DIR"/*-do-tool=$(basename"$i")-setup_tool"$tool"2>/dev/null||continue--merge_tool_path=$(translate_merge_tool_path"$tool")-iftype"$merge_tool_path">/dev/null2>&1-then-available="$available$tool$LF"-else-unavailable="$unavailable$tool$LF"-fi-done-cmd_name=${TOOL_MODE}tool+available=$(filter_tools'mode_ok && is_available''\t\t')+unavailable=$(filter_tools'mode_ok && ! is_available''\t\t')iftest-n"$available"thenecho"'git $cmd_name --tool=<tool>' may be set to one of the following:"-echo"$available"|sort|sed-e's/^/ /'+printf"$available"elseecho"No suitable tool for 'git $cmd_name --tool=<tool>' found."fi
@@ -227,7 +240,7 @@ show_tool_help () {thenechoecho'The following tools are valid, but not currently available:'-echo"$unavailable"|sort|sed-e's/^/ /'+printf"$unavailable"fiiftest-n"$unavailable$available"then
@@ -250,17 +263,12 @@ $toolsprintf"$msg">&2# Loop over each candidate and stop when a valid merge tool is found.-foriin$tools+fortoolin$toolsdo-merge_tool_path=$(translate_merge_tool_path"$i")-iftype"$merge_tool_path">/dev/null2>&1-then-echo"$i"-return0-fi+is_available"$tool"&&echo"$tool"&&return0done-echo>&2"No known merge resolution program available."+echo>&2"No known ${TOOL_MODE} tool is available."return1}
From: Johannes Sixt <hidden> Date: 2016-06-15 22:55:55
Am 27.01.2013 22:24, schrieb David Aguilar:
Use $(command "$arg") instead of "$(command "$arg")" as the latter is
harder to read.
If at all, you should restrict yourself to simplify only variable
assignments. Because this case:
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
cannot work as intended: If the output of $() is empty, then without the
outer quotes this becomes
test -z
without an operand for -z, which is a syntax error (of the test command).
-- Hannes
From: David Aguilar <hidden> Date: 2016-06-15 22:55:55
On Sun, Jan 27, 2013 at 2:08 PM, Johannes Sixt [off-list ref] wrote:
Am 27.01.2013 22:24, schrieb David Aguilar:
quoted
Use $(command "$arg") instead of "$(command "$arg")" as the latter is
harder to read.
If at all, you should restrict yourself to simplify only variable
assignments. Because this case:
quoted
- if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
+ if test -z $(get_merge_tool_cmd "$merge_tool") &&
cannot work as intended: If the output of $() is empty, then without the
outer quotes this becomes
test -z
without an operand for -z, which is a syntax error (of the test command).
Definitely. I learned this the hard way when the tests broke on me while
working it ;-) My patch rewrites things to always use var=$(command)
expressions with separate test "$var" evaluating them.
Thanks for the tip,
--
David
From: Johannes Sixt <hidden> Date: 2016-06-15 22:55:55
Am 27.01.2013 22:24, schrieb David Aguilar:
quoted hunk
Refactor show_tool_help() so that the tool-finding logic is broken out
into separate functions.
Signed-off-by: David Aguilar <redacted>
---
git-mergetool--lib.sh | 60 +++++++++++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 26 deletions(-)
@@ -2,6 +2,33 @@# git-mergetool--lib is a library for common merge tool functionsMERGE_TOOLS_DIR=$(git--exec-path)/mergetools+mode_ok(){+diff_mode&&can_diff||+merge_mode&&can_merge+}
&& and || have the same precedence: if diff_mode and can_diff both are
"true", then the result of the function is that of can_merge. I don't
think that is what is intended.
+filter_tools () {
+ filter="$1"
+ prefix="$2"
+ (
+ cd "$MERGE_TOOLS_DIR" &&
+ for i in *
+ do
+ echo "$i"
+ done
cd "$MERGE_TOOLS_DIR" &&
printf "%s\n" *
But what's wrong with "ls -1"? It would save the explicit sort.
@@ -1,5 +1,6 @@#!/bin/sh# git-mergetool--lib is a library for common merge tool functions+test-z"$MERGE_TOOLS_DIR"&&MERGE_TOOLS_DIR=$(git--exec-path)/mergetools
The preferred pattern in Git seems to be this:
: ${MERGE_TOOLS_DIR=$(git --exec-path)/mergetools}
John
From: John Keeping <hidden> Date: 2016-06-15 22:55:55
On Sun, Jan 27, 2013 at 01:24:45PM -0800, David Aguilar wrote:
+filter_tools () {
+ filter="$1"
+ prefix="$2"
+ (
+ cd "$MERGE_TOOLS_DIR" &&
+ for i in *
+ do
+ echo "$i"
+ done
+ ) | sort | while read tool
+ do
+ setup_tool "$tool" 2>/dev/null &&
+ (eval "$filter" "$tool") &&
+ printf "$prefix$tool\n"
+ done
+}
Can we change this so that it does this:
filter_tools () {
filter="$1"
(
cd "$MERGE_TOOLS_DIR" &&
for i in *
do
echo "$i"
done
) |
while read tool
do
setup_tool "$tool" 2>/dev/null &&
(eval "$filter" "$tool") &&
echo "$tool"
done
}
and keep the sorting and prefix in show_tool_help? This will make it
easier to integrate the user-configured tools from git-config.
John