Tim Henigan <tim.henigan@xxxxxxxxx> 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 <tim.henigan@xxxxxxxxx>
> ---
>
> 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".
> 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?
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html