# [PATCH] mergetools: created new mergetool file for ExamDiff 20 messages from 2016-03-20 to 2016-04-02. Participants: Jacob Nisnevich, Junio C Hamano, David Aguilar. Thread: https://gitlist.dev/t/41758 ## Jacob Nisnevich, 2016-03-20 04:58 Subject: [PATCH] mergetools: created new mergetool file for ExamDiff Message-ID: <1458449932-24342-1-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458449932-24342-1-git-send-email-jacob.nisnevich%40gmail.com ``` --- mergetools/examdiff | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 mergetools/examdiff diff --git a/mergetools/examdiff b/mergetools/examdiff new file mode 100644 index 0000000..474fffe --- /dev/null +++ b/mergetools/examdiff @@ -0,0 +1,37 @@ +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() { + # Use ExamDiff.com if it exists in $PATH + if type -p ExamDiff.com >/dev/null 2>&1 + then + printf ExamDiff.com + return + fi + + # Look for ExamDiff.com in the typical locations + examdiff="ExamDiff Pro/ExamDiff.com" + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) + do + if test -n "$directory" && test -x "$directory/$examdiff" + then + printf '%s' "$directory/$examdiff" + return + fi + done + + printf ExamDiff.com +} \ No newline at end of file -- 1.9.1 ``` ## Junio C Hamano, 2016-03-21 01:02 Subject: Re: [PATCH] mergetools: created new mergetool file for ExamDiff Message-ID: URL: https://gitlist.dev/e/xmqqfuvkod6o.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1458449932-24342-1-git-send-email-jacob.nisnevich@gmail.com> ``` Jacob Nisnevich writes: > --- Missing sign-off. I'll Cc the area expert (David Aguilar). > mergetools/examdiff | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 mergetools/examdiff > > diff --git a/mergetools/examdiff b/mergetools/examdiff > new file mode 100644 > index 0000000..474fffe > --- /dev/null > +++ b/mergetools/examdiff > @@ -0,0 +1,37 @@ > +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() { > + # Use ExamDiff.com if it exists in $PATH > + if type -p ExamDiff.com >/dev/null 2>&1 > + then > + printf ExamDiff.com > + return > + fi > + > + # Look for ExamDiff.com in the typical locations > + examdiff="ExamDiff Pro/ExamDiff.com" > + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | > + cut -d '=' -f 2- | sort -u) > + do > + if test -n "$directory" && test -x "$directory/$examdiff" > + then > + printf '%s' "$directory/$examdiff" > + return > + fi > + done > + > + printf ExamDiff.com This complicated heuristics look like a cut-and-paste from the neighbouring winmerge; makes me suspect that they should share the same helper function to implement the bulk of the above code for better maintainability (e.g. imagine in the future Microsoft decides to introduce another directory organization and makes it necessary to tweak the pattern you give to 'grep -Ei'---WinMergeU user may notice that and fix it, while this script will be overlooked and will stay stale until somebody from examdiff camp do the same fix later). > +} > \ No newline at end of file No newline at end of file? ``` ## David Aguilar, 2016-03-21 03:32 Subject: Re: [PATCH] mergetools: created new mergetool file for ExamDiff Message-ID: <20160321033201.GA2004@gmail.com> URL: https://gitlist.dev/e/20160321033201.GA2004%40gmail.com In-Reply-To: ``` On Sun, Mar 20, 2016 at 06:02:55PM -0700, Junio C Hamano wrote: > Jacob Nisnevich writes: > > > --- > > Missing sign-off. > > I'll Cc the area expert (David Aguilar). > > > mergetools/examdiff | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > create mode 100644 mergetools/examdiff > > > > diff --git a/mergetools/examdiff b/mergetools/examdiff > > new file mode 100644 > > index 0000000..474fffe > > --- /dev/null > > +++ b/mergetools/examdiff > > @@ -0,0 +1,37 @@ > > +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() { > > + # Use ExamDiff.com if it exists in $PATH > > + if type -p ExamDiff.com >/dev/null 2>&1 > > + then > > + printf ExamDiff.com > > + return > > + fi > > + > > + # Look for ExamDiff.com in the typical locations > > + examdiff="ExamDiff Pro/ExamDiff.com" > > + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | > > + cut -d '=' -f 2- | sort -u) > > + do > > + if test -n "$directory" && test -x "$directory/$examdiff" > > + then > > + printf '%s' "$directory/$examdiff" > > + return > > + fi > > + done > > + > > + printf ExamDiff.com > > This complicated heuristics look like a cut-and-paste from the > neighbouring winmerge; makes me suspect that they should share the > same helper function to implement the bulk of the above code for > better maintainability (e.g. imagine in the future Microsoft decides > to introduce another directory organization and makes it necessary > to tweak the pattern you give to 'grep -Ei'---WinMergeU user may > notice that and fix it, while this script will be overlooked and > will stay stale until somebody from examdiff camp do the same fix > later). I agree with that. Something like mergetool_find_win32_cmd() might make sense as a helper function that we can reuse here. > > +} > > \ No newline at end of file > > No newline at end of file? Using sublime text perhaps? It defaults to not including the final line newline terminator. https://forum.sublimetext.com/t/make-saving-newline-at-eof-the-installation-default/9842 If so, please configure it as detailed in the above thread. cheers, -- David ``` ## Jacob Nisnevich, 2016-03-23 00:43 Subject: [PATCH 0/2] Helper function for ExamDiff and Winmerge mergetools Message-ID: <1458693794-9124-1-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458693794-9124-1-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <20160321033201.GA2004@gmail.com> ``` I've been trying to implement a helper function for both Winmerge and ExamDiff as you requested, but for some reason including another shell script with . or source seems to fail on Windows in this case. I've attached the patches for the two commits I've made so far. Is there anything I'm doing wrong or is it an issue with Windows? Jacob Nisnevich (2): mergetools: created new mergetool file for ExamDiff created helper function for both winmerge and examdiff mergetools ...s-created-new-mergetool-file-for-ExamDiff.patch | 58 ++++++++++++++++++++++ mergetools/examdiff | 20 ++++++++ mergetools/mergetools_helpers | 30 +++++++++++ mergetools/winmerge | 23 ++------- 4 files changed, 111 insertions(+), 20 deletions(-) create mode 100644 0001-mergetools-created-new-mergetool-file-for-ExamDiff.patch create mode 100644 mergetools/examdiff create mode 100644 mergetools/mergetools_helpers -- 1.9.1 ``` ## Jacob Nisnevich, 2016-03-23 00:43 Subject: [PATCH 1/2] mergetools: created new mergetool file for ExamDiff Message-ID: <1458693794-9124-2-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458693794-9124-2-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458693794-9124-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- mergetools/examdiff | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 mergetools/examdiff diff --git a/mergetools/examdiff b/mergetools/examdiff new file mode 100644 index 0000000..474fffe --- /dev/null +++ b/mergetools/examdiff @@ -0,0 +1,37 @@ +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() { + # Use ExamDiff.com if it exists in $PATH + if type -p ExamDiff.com >/dev/null 2>&1 + then + printf ExamDiff.com + return + fi + + # Look for ExamDiff.com in the typical locations + examdiff="ExamDiff Pro/ExamDiff.com" + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) + do + if test -n "$directory" && test -x "$directory/$examdiff" + then + printf '%s' "$directory/$examdiff" + return + fi + done + + printf ExamDiff.com +} \ No newline at end of file -- 1.9.1 ``` ## Jacob Nisnevich, 2016-03-23 00:43 Subject: [PATCH 2/2] created helper function for both winmerge and examdiff mergetools Message-ID: <1458693794-9124-3-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458693794-9124-3-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458693794-9124-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- ...s-created-new-mergetool-file-for-ExamDiff.patch | 58 ++++++++++++++++++++++ mergetools/examdiff | 25 ++-------- mergetools/mergetools_helpers | 30 +++++++++++ mergetools/winmerge | 23 ++------- 4 files changed, 95 insertions(+), 41 deletions(-) create mode 100644 0001-mergetools-created-new-mergetool-file-for-ExamDiff.patch create mode 100644 mergetools/mergetools_helpers diff --git a/0001-mergetools-created-new-mergetool-file-for-ExamDiff.patch b/0001-mergetools-created-new-mergetool-file-for-ExamDiff.patch new file mode 100644 index 0000000..99e0d6b --- /dev/null +++ b/0001-mergetools-created-new-mergetool-file-for-ExamDiff.patch @@ -0,0 +1,58 @@ +From a26ad589bc5e747962359c89c5536858748e5eb8 Mon Sep 17 00:00:00 2001 +From: Jacob Nisnevich +Date: Sat, 19 Mar 2016 17:31:50 -0700 +Subject: [PATCH] mergetools: created new mergetool file for ExamDiff +To: git@vger.kernel.org + +--- + mergetools/examdiff | 37 +++++++++++++++++++++++++++++++++++++ + 1 file changed, 37 insertions(+) + create mode 100644 mergetools/examdiff + +diff --git a/mergetools/examdiff b/mergetools/examdiff +new file mode 100644 +index 0000000..474fffe +--- /dev/null ++++ b/mergetools/examdiff +@@ -0,0 +1,37 @@ ++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() { ++ # Use ExamDiff.com if it exists in $PATH ++ if type -p ExamDiff.com >/dev/null 2>&1 ++ then ++ printf ExamDiff.com ++ return ++ fi ++ ++ # Look for ExamDiff.com in the typical locations ++ examdiff="ExamDiff Pro/ExamDiff.com" ++ for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | ++ cut -d '=' -f 2- | sort -u) ++ do ++ if test -n "$directory" && test -x "$directory/$examdiff" ++ then ++ printf '%s' "$directory/$examdiff" ++ return ++ fi ++ done ++ ++ printf ExamDiff.com ++} +\ No newline at end of file +-- +1.9.1 + diff --git a/mergetools/examdiff b/mergetools/examdiff index 474fffe..8b66c17 100644 --- a/mergetools/examdiff +++ b/mergetools/examdiff @@ -1,3 +1,5 @@ +. mergetools_helpers + diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" -nh } @@ -14,24 +16,5 @@ merge_cmd () { } translate_merge_tool_path() { - # Use ExamDiff.com if it exists in $PATH - if type -p ExamDiff.com >/dev/null 2>&1 - then - printf ExamDiff.com - return - fi - - # Look for ExamDiff.com in the typical locations - examdiff="ExamDiff Pro/ExamDiff.com" - for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | - cut -d '=' -f 2- | sort -u) - do - if test -n "$directory" && test -x "$directory/$examdiff" - then - printf '%s' "$directory/$examdiff" - return - fi - done - - printf ExamDiff.com -} \ No newline at end of file + mergetool_find_win32_cmd "ExamDiff.com" "ExamDiff Pro" +} diff --git a/mergetools/mergetools_helpers b/mergetools/mergetools_helpers new file mode 100644 index 0000000..6df9d09 --- /dev/null +++ b/mergetools/mergetools_helpers @@ -0,0 +1,30 @@ +# Find path to win32 executable using typical locations +# Arguments +# executable - default name of executable file +# folder - folder containing executable file from Program Files +# Returns +# Path to the executable +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 + 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..265d853 100644 --- a/mergetools/winmerge +++ b/mergetools/winmerge @@ -1,3 +1,5 @@ +. mergetools_helpers + 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 ``` ## Junio C Hamano, 2016-03-23 02:41 Subject: Re: [PATCH 2/2] created helper function for both winmerge and examdiff mergetools Message-ID: URL: https://gitlist.dev/e/xmqqpoulex0b.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1458693794-9124-3-git-send-email-jacob.nisnevich@gmail.com> ``` Jacob Nisnevich writes: > diff --git a/mergetools/examdiff b/mergetools/examdiff > index 474fffe..8b66c17 100644 > --- a/mergetools/examdiff > +++ b/mergetools/examdiff > @@ -1,3 +1,5 @@ > +. mergetools_helpers > + The way dot-inclusion is done in existing mergetools/* files may give you a hint, perhaps? $ git grep '^\. ' mergetools mergetools/bc3:. "$MERGE_TOOLS_DIR/bc" mergetools/gvimdiff:. "$MERGE_TOOLS_DIR/vimdiff" mergetools/gvimdiff2:. "$MERGE_TOOLS_DIR/vimdiff" mergetools/gvimdiff3:. "$MERGE_TOOLS_DIR/vimdiff" mergetools/vimdiff2:. "$MERGE_TOOLS_DIR/vimdiff" mergetools/vimdiff3:. "$MERGE_TOOLS_DIR/vimdiff" ``` ## Jacob Nisnevich, 2016-03-23 22:55 Subject: [PATCH] mergetools: implemented new mergetool file for ExamDiff Message-ID: <1458773745-783-1-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458773745-783-1-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: ``` Thanks for the hint. I used the other examples to fix the implementation of the ExamDiff mergetool. I've tested the Winmerge and ExamDiff mergetools with both `git difftool` and `git mergetool` and both seem to work as expected. I've combined all my changes into one patch. Are there any other issues with my changes? Jacob Nisnevich (1): mergetools: implemented new mergetool file for ExamDiff 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 -- 1.9.1 ``` ## Jacob Nisnevich, 2016-03-23 22:55 Subject: [PATCH] mergetools: implemented new mergetool file for ExamDiff Message-ID: <1458773745-783-2-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458773745-783-2-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458773745-783-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- 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 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 @@ -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 + 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" + 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 Aguilar, 2016-03-24 07:44 Subject: Re: [PATCH] mergetools: implemented new mergetool file for ExamDiff Message-ID: <20160324074404.GA30574@gmail.com> URL: https://gitlist.dev/e/20160324074404.GA30574%40gmail.com In-Reply-To: <1458773745-783-2-git-send-email-jacob.nisnevich@gmail.com> ``` On Wed, Mar 23, 2016 at 03:55:45PM -0700, Jacob Nisnevich wrote: > Signed-off-by: Jacob Nisnevich > --- 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. > 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(). > @@ -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. > + 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. > 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 ``` ## Junio C Hamano, 2016-03-24 15:43 Subject: Re: [PATCH] mergetools: implemented new mergetool file for ExamDiff Message-ID: URL: https://gitlist.dev/e/xmqq60wbc24h.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20160324074404.GA30574@gmail.com> ``` David Aguilar writes: > 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(). Thanks for a clearly described set of rules that people can easily follow. ``` ## Jacob Nisnevich, 2016-03-25 22:52 Subject: [PATCH 0/2] add support for ExamDiff Message-ID: <1458946376-30296-1-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458946376-30296-1-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: ``` I implemented all the changes you suggested and separated the patch into two different patches. Thanks for all the help David. Jacob Nisnevich (2): mergetools: create mergetool_find_win32_cmd() helper function for winmerge mergetools: add support for ExamDiff git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ mergetools/examdiff | 18 ++++++++++++++++++ mergetools/winmerge | 21 +-------------------- 3 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 mergetools/examdiff -- 1.9.1 ``` ## Jacob Nisnevich, 2016-03-25 22:52 Subject: [PATCH 1/2] mergetools: create mergetool_find_win32_cmd() helper function for winmerge Message-ID: <1458946376-30296-2-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458946376-30296-2-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458946376-30296-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ mergetools/winmerge | 21 +-------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 54ac8e4..c5fa820 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -372,3 +372,28 @@ get_merge_tool () { fi echo "$merge_tool" } + +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 '%s' $executable + 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 '%s' $executable +} diff --git a/mergetools/winmerge b/mergetools/winmerge index 74a66d4..f3819d3 100644 --- a/mergetools/winmerge +++ b/mergetools/winmerge @@ -13,24 +13,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 ``` ## Jacob Nisnevich, 2016-03-25 22:52 Subject: [PATCH 2/2] mergetools: add support for ExamDiff Message-ID: <1458946376-30296-3-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458946376-30296-3-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458946376-30296-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- mergetools/examdiff | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 mergetools/examdiff diff --git a/mergetools/examdiff b/mergetools/examdiff new file mode 100644 index 0000000..7b524d4 --- /dev/null +++ b/mergetools/examdiff @@ -0,0 +1,18 @@ +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" +} -- 1.9.1 ``` ## Junio C Hamano, 2016-03-25 23:00 Subject: Re: [PATCH 1/2] mergetools: create mergetool_find_win32_cmd() helper function for winmerge Message-ID: URL: https://gitlist.dev/e/xmqqio0a17ux.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1458946376-30296-2-git-send-email-jacob.nisnevich@gmail.com> ``` Jacob Nisnevich writes: > Signed-off-by: Jacob Nisnevich > --- > git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ > mergetools/winmerge | 21 +-------------------- > 2 files changed, 26 insertions(+), 20 deletions(-) > > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 54ac8e4..c5fa820 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -372,3 +372,28 @@ get_merge_tool () { > fi > echo "$merge_tool" > } > + > +mergetool_find_win32_cmd () { > + executable=$1 > + folder=$2 > + > + # Use executable.com if it exists in $PATH s/executable.com/$executable/ > + if type -p $executable >/dev/null 2>&1 This needs to be quoted, if type -p "$executable" >/dev/null 2>&1 > + then > + printf '%s' $executable Likewise. > + 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 '%s' $executable Likewise. Other than these points, I do not see anything wrong in this patch. Thanks. By the way, "directory/folder/stuff" sounds somewhat strange, no? ``` ## Jacob Nisnevich, 2016-03-25 23:17 Subject: [PATCH 0/2] mergetools: add support for ExamDiff Message-ID: <1458947877-31764-1-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458947877-31764-1-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: ``` OK I add the quotes and modified the comment. I also changed $folder to $sub_directory. I think that makes a little bit more sense and sounds a lot better. Jacob Nisnevich (2): mergetools: create mergetool_find_win32_cmd() helper function for winmerge mergetools: add support for ExamDiff git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ mergetools/examdiff | 18 ++++++++++++++++++ mergetools/winmerge | 21 +-------------------- 3 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 mergetools/examdiff -- 1.9.1 ``` ## Jacob Nisnevich, 2016-03-25 23:17 Subject: [PATCH 1/2] mergetools: create mergetool_find_win32_cmd() helper function for winmerge Message-ID: <1458947877-31764-2-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458947877-31764-2-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458947877-31764-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ mergetools/winmerge | 21 +-------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 54ac8e4..302c56d 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -372,3 +372,28 @@ get_merge_tool () { fi echo "$merge_tool" } + +mergetool_find_win32_cmd () { + executable=$1 + sub_directory=$2 + + # Use $executable if it exists in $PATH + if type -p "$executable" >/dev/null 2>&1 + then + printf '%s' "$executable" + 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/$sub_directory/$executable" + then + printf '%s' "$directory/$sub_directory/$executable" + return + fi + done + + printf '%s' "$executable" +} diff --git a/mergetools/winmerge b/mergetools/winmerge index 74a66d4..f3819d3 100644 --- a/mergetools/winmerge +++ b/mergetools/winmerge @@ -13,24 +13,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 ``` ## Jacob Nisnevich, 2016-03-25 23:17 Subject: [PATCH 2/2] mergetools: add support for ExamDiff Message-ID: <1458947877-31764-3-git-send-email-jacob.nisnevich@gmail.com> URL: https://gitlist.dev/e/1458947877-31764-3-git-send-email-jacob.nisnevich%40gmail.com In-Reply-To: <1458947877-31764-1-git-send-email-jacob.nisnevich@gmail.com> ``` Signed-off-by: Jacob Nisnevich --- mergetools/examdiff | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 mergetools/examdiff diff --git a/mergetools/examdiff b/mergetools/examdiff new file mode 100644 index 0000000..7b524d4 --- /dev/null +++ b/mergetools/examdiff @@ -0,0 +1,18 @@ +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" +} -- 1.9.1 ``` ## Junio C Hamano, 2016-04-01 18:10 Subject: Re: [PATCH 0/2] mergetools: add support for ExamDiff Message-ID: URL: https://gitlist.dev/e/xmqqfuv5i4i7.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1458947877-31764-1-git-send-email-jacob.nisnevich@gmail.com> ``` Jacob Nisnevich writes: > OK I add the quotes and modified the comment. I also changed $folder to > $sub_directory. I think that makes a little bit more sense and sounds a lot > better. > > Jacob Nisnevich (2): > mergetools: create mergetool_find_win32_cmd() helper function for > winmerge > mergetools: add support for ExamDiff > > git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ > mergetools/examdiff | 18 ++++++++++++++++++ > mergetools/winmerge | 21 +-------------------- > 3 files changed, 44 insertions(+), 20 deletions(-) > create mode 100644 mergetools/examdiff This round looked good to me. David, does this look sensible to you? Thanks. ``` ## David Aguilar, 2016-04-02 21:02 Subject: Re: [PATCH 0/2] mergetools: add support for ExamDiff Message-ID: <20160402210239.GA6917@gmail.com> URL: https://gitlist.dev/e/20160402210239.GA6917%40gmail.com In-Reply-To: ``` On Fri, Apr 01, 2016 at 11:10:56AM -0700, Junio C Hamano wrote: > Jacob Nisnevich writes: > > > OK I add the quotes and modified the comment. I also changed $folder to > > $sub_directory. I think that makes a little bit more sense and sounds a lot > > better. > > > > Jacob Nisnevich (2): > > mergetools: create mergetool_find_win32_cmd() helper function for > > winmerge > > mergetools: add support for ExamDiff > > > > git-mergetool--lib.sh | 25 +++++++++++++++++++++++++ > > mergetools/examdiff | 18 ++++++++++++++++++ > > mergetools/winmerge | 21 +-------------------- > > 3 files changed, 44 insertions(+), 20 deletions(-) > > create mode 100644 mergetools/examdiff > > This round looked good to me. > David, does this look sensible to you? > > Thanks. Yes, thank you. Acked-by: David Aguilar cheers, -- David ```