Re: [PATCH] mergetools: implemented new mergetool file for ExamDiff
From: David Aguilar <hidden>
Date: 2016-06-15 23:08:55
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] mergetools: implemented new mergetool file for ExamDiff · Junio C Hamano <hidden>
On Wed, Mar 23, 2016 at 03:55:45PM -0700, Jacob Nisnevich wrote:
Signed-off-by: Jacob Nisnevich <redacted> ---
Please write commit message in an imperative tone. e.g. "mergetools: add support for ExamDiff" might be a good summary.
mergetools/examdiff | 20 ++++++++++++++++++++ mergetools/mergetools_helpers | 24 ++++++++++++++++++++++++ mergetools/winmerge | 23 +++-------------------- 3 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 mergetools/examdiff create mode 100644 mergetools/mergetools_helpers
This patch is doing two things. It's probably worth breaking this patch up into two parts. Patch 1 can add add the new helper function and update winmerge to use it. Patch 2 can add the new examdiff helper.
quoted hunk ↗ jump to hunk
diff --git a/mergetools/examdiff b/mergetools/examdiff new file mode 100644 index 0000000..c5edd0e --- /dev/null +++ b/mergetools/examdiff@@ -0,0 +1,20 @@ +. "$MERGE_TOOLS_DIR/mergetools_helpers" + +diff_cmd () { + "$merge_tool_path" "$LOCAL" "$REMOTE" -nh +} + +merge_cmd () { + touch "$BACKUP" + if $base_present + then + "$merge_tool_path" -merge "$LOCAL" "$BASE" "$REMOTE" -o:"$MERGED" -nh + else + "$merge_tool_path" -merge "$LOCAL" "$REMOTE" -o:"$MERGED" -nh + fi + check_unchanged +} + +translate_merge_tool_path() { + mergetool_find_win32_cmd "ExamDiff.com" "ExamDiff Pro" +}diff --git a/mergetools/mergetools_helpers b/mergetools/mergetools_helpers new file mode 100644 index 0000000..46ae2d8 --- /dev/null +++ b/mergetools/mergetools_helpers
We can probably do this without introducing a new file. One possible home for this is with the rest of the "default" definitions of the functions in git-mergetool--lib.sh's setup_tool() function. But, that hints that we expect tools to override it. A better place would be as a normal function inside git-mergetool--lib.sh, which more strongly hints that we do not expect tools to override mergetool_find_win32_cmd().
quoted hunk ↗ jump to hunk
@@ -0,0 +1,24 @@ +mergetool_find_win32_cmd () { + executable=$1 + folder=$2 + + # Use executable.com if it exists in $PATH + if type -p $executable >/dev/null 2>&1 + then + printf $executable
It might nut hurt to write this as, printf '%s' "$executable" For consistency and future-proofing.
quoted hunk ↗ jump to hunk
+ return + fi + + # Look for executable in the typical locations + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) + do + if test -n "$directory" && test -x "$directory/$folder/$executable" + then + printf '%s' "$directory/$folder/$executable" + return + fi + done + + printf $executable +}diff --git a/mergetools/winmerge b/mergetools/winmerge index 74a66d4..c785be8 100644 --- a/mergetools/winmerge +++ b/mergetools/winmerge@@ -1,3 +1,5 @@ +. "$MERGE_TOOLS_DIR/mergetools_helpers" +
If we move the definition into mergetool--lib then we do not need to dot-include any files here -- it'll simply be present and available.
quoted hunk ↗ jump to hunk
diff_cmd () { "$merge_tool_path" -u -e "$LOCAL" "$REMOTE" return 0@@ -13,24 +15,5 @@ merge_cmd () { } translate_merge_tool_path() { - # Use WinMergeU.exe if it exists in $PATH - if type -p WinMergeU.exe >/dev/null 2>&1 - then - printf WinMergeU.exe - return - fi - - # Look for WinMergeU.exe in the typical locations - winmerge_exe="WinMerge/WinMergeU.exe" - for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | - cut -d '=' -f 2- | sort -u) - do - if test -n "$directory" && test -x "$directory/$winmerge_exe" - then - printf '%s' "$directory/$winmerge_exe" - return - fi - done - - printf WinMergeU.exe + mergetool_find_win32_cmd "WinMergeU.exe" "WinMerge" }-- 1.9.1
-- David