Re: [PATCH] mergetools: created new mergetool file for ExamDiff

[David Aguilar <davvid@gmail.com>]

On Sun, Mar 20, 2016 at 06:02:55PM -0700, Junio C Hamano wrote:
> Jacob Nisnevich <jacob.nisnevich@gmail.com> 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