Re: [PATCH] mergetools: add support for DeltaWalker
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:11
Tim Henigan [off-list ref] writes:
Since bc7a96a8965d, integration with external diff/merge tools requires a mergetools/$tool file to be present. This commits adds support for DeltaWalker. Signed-off-by: Tim Henigan <redacted> --- Tested with DeltaWalker v1.9.8 on Ubuntu 11.10 and msysgit on Win7. mergetools/DeltaWalker | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 mergetools/DeltaWalker
I see that the earlier refactoring to make mergetool backend pluggable is starting to pay off rather nicely. It is not "since ..., requires ...", but "thanks to ..., adding a random new tool is just a matter of dropping a trivial shell snippet in the directory".
quoted hunk
diff --git a/mergetools/DeltaWalker b/mergetools/DeltaWalker new file mode 100644 index 0000000..ae66686 --- /dev/null +++ b/mergetools/DeltaWalker@@ -0,0 +1,13 @@ +diff_cmd () { + "$merge_tool_path" "$LOCAL" "$REMOTE" >/dev/null 2>&1 +}
I noticed that most if not all of the mergetool plug-ins have these ugly /dev/null redirects everywhere, and I also suspect that this may be inherited from the first bad instance with copy-and-paste. I was about to suggest doing these redirection on the calling side in git-mergetool.sh but there are some that do not want them, so perhaps each plug-in needs to decide if they want redirection individually.
+merge_cmd () {
+ if $base_present
+ then
+ "$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" -merged="$PWD/$MERGED" >/dev/null 2>&1
+ else
+ "$merge_tool_path" "$LOCAL" "$REMOTE" -merged="$PWD/$MERGED" >/dev/null 2>&1
+ fi
Perhaps doing the above like this might make it a bit less of an eye-sore.
if $base_present
then
"$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" -merged="$PWD/$MERGED"
else
"$merge_tool_path" "$LOCAL" "$REMOTE" -merged="$PWD/$MERGED"
fi >/dev/null 2>&1
+ status=$?
This is highly dubious. Looking at existing mergetools/*, I think the caller expects merge_cmd to signal success or failure with $?, so you probably just want to drop this line; the caller will then get the $? that was set by the "$merge_tool_path" command. That is how your diff_cmd is communicating with its caller after all, no?
+}