Refactor git-mergetool--lib.sh

PATCH In review David Aguilar 4 participants 18 messages Jun 15, 2016 Kbuild

Share & tools

David Aguilar

author

2016-06-15 22:51 UTC #0 lore raw

cover Refactor git-mergetool--lib.sh

This series splits the git-mergetool--lib.sh file into separate tool scriptlets that define the commands for diff and merge.

David Aguilar (4):
difftool--helper: Make style consistent with git
mergetool--lib: Make style consistent with git
mergetool--lib: Refactor tools into separate files mergetools/meld: Use '--output' when available

 Makefile                 |   11 ++
 git-difftool--helper.sh  |   18 ++-
 git-mergetool--lib.sh    |  394 ++++++++++------------------------------------
 mergetools/araxis        |   21 +++
 mergetools/bc3           |   20 +++
 mergetools/defaults      |   46 ++++++
 mergetools/diffuse       |   17 ++
 mergetools/ecmerge       |   16 ++
 mergetools/emerge        |   23 +++
 mergetools/kdiff3        |   24 +++
 mergetools/kompare       |    7 +
 mergetools/meld          |   40 +++++
 mergetools/opendiff      |   16 ++
 mergetools/p4merge       |   10 ++
 mergetools/tkdiff        |   12 ++
 mergetools/tortoisemerge |   17 ++
 mergetools/vim           |   44 +++++
 mergetools/xxdiff        |   25 +++
 18 files changed, 441 insertions(+), 320 deletions(-)
 create mode 100644 mergetools/araxis
 create mode 100644 mergetools/bc3
 create mode 100644 mergetools/defaults
 create mode 100644 mergetools/diffuse
 create mode 100644 mergetools/ecmerge
 create mode 100644 mergetools/emerge
 create mode 100644 mergetools/kdiff3
 create mode 100644 mergetools/kompare
 create mode 100644 mergetools/meld
 create mode 100644 mergetools/opendiff
 create mode 100644 mergetools/p4merge
 create mode 100644 mergetools/tkdiff
 create mode 100644 mergetools/tortoisemerge
 create mode 100644 mergetools/vim
 create mode 100644 mergetools/xxdiff

$ git describe --dirty
v1.7.6-476-g572925c
$ git version
git version 1.7.6.476.g57292
$ ./t7610-mergetool.sh 
ok 1 - setup
ok 2 - custom mergetool
ok 3 - mergetool crlf
ok 4 - mergetool in subdir
ok 5 - mergetool on file in parent dir
ok 6 - mergetool skips autoresolved
ok 7 - mergetool merges all from subdir
ok 8 - mergetool skips resolved paths when rerere is active
ok 9 - deleted vs modified submodule
ok 10 - file vs modified submodule
ok 11 - submodule in subdirectory
ok 12 - directory vs modified submodule
# passed all 12 test(s)
1..12
$ ./t7800-difftool.sh 
ok 1 - setup
ok 2 - custom commands
ok 3 - difftool ignores bad --tool values
ok 4 - difftool honors --gui
ok 5 - difftool --gui works without configured diff.guitool
ok 6 - GIT_DIFF_TOOL variable
ok 7 - GIT_DIFF_TOOL overrides
ok 8 - GIT_DIFFTOOL_NO_PROMPT variable
ok 9 - GIT_DIFFTOOL_PROMPT variable
ok 10 - difftool.prompt config variable is false
ok 11 - difftool merge.prompt = false
ok 12 - difftool.prompt can overridden with -y
ok 13 - difftool.prompt can overridden with --prompt
ok 14 - difftool last flag wins
ok 15 - difftool + mergetool config variables
ok 16 - difftool.<tool>.path
ok 17 - difftool --extcmd=cat
ok 18 - difftool --extcmd cat
ok 19 - difftool -x cat
ok 20 - difftool --extcmd echo arg1
ok 21 - difftool --extcmd cat arg1
ok 22 - difftool --extcmd cat arg2
# passed all 22 test(s)
1..22

Signature · 1 line

1.7.6.476.g57292

David Aguilar

author

2016-06-15 22:51 UTC #1 lore raw

PATCH 1/4 difftool--helper: Make style consistent with git

Use the predominant conditional style where "then" appears alone on the line after the test expression.

Signed-off-by David Aguilar <[email protected]>

---
 git-difftool--helper.sh |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

git-difftool--helper.sh +12 -6

index 0594bf7..8452890 100755
@@ -13,7 +13,8 @@ TOOL_MODE=diff
 should_prompt () {
 	prompt_merge=$(git config --bool mergetool.prompt || echo true)
 	prompt=$(git config --bool difftool.prompt || echo $prompt_merge)
-	if test "$prompt" = true; then
+	if test "$prompt" = true
+	then
 		test -z "$GIT_DIFFTOOL_NO_PROMPT"
 	else
 		test -n "$GIT_DIFFTOOL_PROMPT"

@@ -37,9 +38,11 @@ launch_merge_tool () {
 
 	# $LOCAL and $REMOTE are temporary files so prompt
 	# the user with the real $MERGED name before launching $merge_tool.
-	if should_prompt; then
+	if should_prompt
+	then
 		printf "\nViewing: '$MERGED'\n"
-		if use_ext_cmd; then
+		if use_ext_cmd
+		then
 			printf "Hit return to launch '%s': " \
 				"$GIT_DIFFTOOL_EXTCMD"
 		else

@@ -48,7 +51,8 @@ launch_merge_tool () {
 		read ans
 	fi
 
-	if use_ext_cmd; then
+	if use_ext_cmd
+	then
 		export BASE
 		eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
 	else

@@ -56,8 +60,10 @@ launch_merge_tool () {
 	fi
 }
 
-if ! use_ext_cmd; then
-	if test -n "$GIT_DIFF_TOOL"; then
+if ! use_ext_cmd
+then
+	if test -n "$GIT_DIFF_TOOL"
+	then
 		merge_tool="$GIT_DIFF_TOOL"
 	else
 		merge_tool="$(get_merge_tool)" || exit

Signature · 1 line

1.7.6.476.g57292

Sebastian Schuberth

2016-06-15 22:51 UTC #2 lore raw

Re: [PATCH 1/4] difftool--helper: Make style consistent with git

Quoted 2 lines · David Aguilar

Use the predominant conditional style where "then" appears alone on the line after the test expression.

I support your effort to unify the style, but for my personal taste it has gone the wrong way :-) Even if "then" on its own line was the predominant style in the merge scripts, I would have voted for putting all "then" in the same line as "if" as I find this easier to read and comprehend. But then again it's obvious that such a change would polarize ;-)

Signature · 1 line

Sebastian Schuberth

David Aguilar

author

2016-06-15 22:51 UTC #3 lore raw

Re: [PATCH 1/4] difftool--helper: Make style consistent with git

Quoted 5 lines · Sebastian Schuberth

Quoted 2 lines · David Aguilar
Use the predominant conditional style where "then" appears alone on the line after the test expression.

Quoted 9 lines

I support your effort to unify the style, but for my personal taste it has gone the wrong way :-) Even if "then" on its own line was the predominant style in the merge scripts, I would have voted for putting all "then" in the same line as "if" as I find this easier to read and comprehend. But then again it's obvious that such a change would polarize ;-)
Signature · 1 line
Sebastian Schuberth

I am fine reworking it either way.
There's not a specific note about it in
Documentation/CodingStyle.

If someone wants to make the final call I can patch the docs and rework these patches as needed.

Thanks,

Signature · 1 line

					David

Junio C Hamano

2016-06-15 22:51 UTC #4 lore raw

Re: [PATCH 1/4] difftool--helper: Make style consistent with git

Quoted 6 lines · David Aguilar

I am fine reworking it either way.
There's not a specific note about it in
Documentation/CodingStyle.
If someone wants to make the final call I can patch the docs and rework these patches as needed.

Please stick to (and document if not):

	if test ...
        then
        	action
	elif condition that can become looong so more &&
        	condition needs to be split to next line &&
                even more condition
	then
		action
	else
		action
	fi

Thanks.

You can think of it as "war against semicolon" if you want.

David Aguilar

author

2016-06-15 22:51 UTC #5 lore raw

PATCH 2/4 mergetool--lib: Make style consistent with git

In reply to David Aguilar

Use the predominant conditional style where "then" appears alone on the line after the test expression. Remove spaces after ">" output redirections. Remove unnecessary parentheses around the kdiff3 commands.

Signed-off-by David Aguilar <[email protected]>

---
 git-mergetool--lib.sh |  153 ++++++++++++++++++++++++++++++++-----------------
 1 files changed, 100 insertions(+), 53 deletions(-)

git-mergetool--lib.sh +100 ~~-53~~

index 91f90ac..03cfb19 100644
@@ -32,10 +32,12 @@ translate_merge_tool_path () {
 }
 
 check_unchanged () {
-	if test "$MERGED" -nt "$BACKUP"; then
+	if test "$MERGED" -nt "$BACKUP"
+	then
 		status=0
 	else
-		while true; do
+		while true
+		do
 			echo "$MERGED seems unchanged."
 			printf "Was the merge successful? [y/n] "
 			read answer

@@ -53,17 +55,20 @@ valid_tool () {
 	kdiff3 | meld | opendiff | p4merge | tkdiff | vimdiff | vimdiff2 | xxdiff)
 		;; # happy
 	kompare)
-		if ! diff_mode; then
+		if ! diff_mode
+		then
 			return 1
 		fi
 		;;
 	tortoisemerge)
-		if ! merge_mode; then
+		if ! merge_mode
+		then
 			return 1
 		fi
 		;;
 	*)
-		if test -z "$(get_merge_tool_cmd "$1")"; then
+		if test -z "$(get_merge_tool_cmd "$1")"
+		then
 			return 1
 		fi
 		;;

@@ -72,12 +77,14 @@ valid_tool () {
 
 get_merge_tool_cmd () {
 	# Prints the custom command for a merge tool
-	if test -n "$1"; then
+	if test -n "$1"
+	then
 		merge_tool="$1"
 	else
 		merge_tool="$(get_merge_tool)"
 	fi
-	if diff_mode; then
+	if diff_mode
+	then
 		echo "$(git config difftool.$merge_tool.cmd ||
 		        git config mergetool.$merge_tool.cmd)"
 	else

@@ -97,9 +104,11 @@ run_merge_tool () {
 
 	case "$1" in
 	araxis)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" -wait -merge -3 -a1 \
 					"$BASE" "$LOCAL" "$REMOTE" "$MERGED" \
 					>/dev/null 2>&1

@@ -115,9 +124,11 @@ run_merge_tool () {
 		fi
 		;;
 	bc3)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
 					-mergeoutput="$MERGED"
 			else

@@ -130,9 +141,11 @@ run_merge_tool () {
 		fi
 		;;
 	diffuse)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" \
 					"$LOCAL" "$MERGED" "$REMOTE" \
 					"$BASE" | cat

@@ -146,9 +159,11 @@ run_merge_tool () {
 		fi
 		;;
 	ecmerge)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
 					--default --mode=merge3 --to="$MERGED"
 			else

@@ -162,8 +177,10 @@ run_merge_tool () {
 		fi
 		;;
 	emerge)
-		if merge_mode; then
-			if $base_present; then
+		if merge_mode
+		then
+			if $base_present
+			then
 				"$merge_tool_path" \
 					-f emerge-files-with-ancestor-command \
 					"$LOCAL" "$REMOTE" "$BASE" \

@@ -181,9 +198,11 @@ run_merge_tool () {
 		fi
 		;;
 	gvimdiff|vimdiff)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" -f -d -c "wincmd J" \
 					"$MERGED" "$LOCAL" "$BASE" "$REMOTE"
 			else

@@ -198,7 +217,8 @@ run_merge_tool () {
 		fi
 		;;
 	gvimdiff2|vimdiff2)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
 			"$merge_tool_path" -f -d -c "wincmd l" \
 				"$LOCAL" "$MERGED" "$REMOTE"

@@ -210,36 +230,39 @@ run_merge_tool () {
 		fi
 		;;
 	kdiff3)
-		if merge_mode; then
-			if $base_present; then
-				("$merge_tool_path" --auto \
+		if merge_mode
+		then
+			if $base_present
+			then
+				"$merge_tool_path" --auto \
 					--L1 "$MERGED (Base)" \
 					--L2 "$MERGED (Local)" \
 					--L3 "$MERGED (Remote)" \
 					-o "$MERGED" \
 					"$BASE" "$LOCAL" "$REMOTE" \
-				> /dev/null 2>&1)
+				>/dev/null 2>&1
 			else
-				("$merge_tool_path" --auto \
+				"$merge_tool_path" --auto \
 					--L1 "$MERGED (Local)" \
 					--L2 "$MERGED (Remote)" \
 					-o "$MERGED" \
 					"$LOCAL" "$REMOTE" \
-				> /dev/null 2>&1)
+				>/dev/null 2>&1
 			fi
 			status=$?
 		else
-			("$merge_tool_path" --auto \
+			"$merge_tool_path" --auto \
 				--L1 "$MERGED (A)" \
 				--L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
-			> /dev/null 2>&1)
+			>/dev/null 2>&1
 		fi
 		;;
 	kompare)
 		"$merge_tool_path" "$LOCAL" "$REMOTE"
 		;;
 	meld)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
 			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged

@@ -248,9 +271,11 @@ run_merge_tool () {
 		fi
 		;;
 	opendiff)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" "$LOCAL" "$REMOTE" \
 					-ancestor "$BASE" \
 					-merge "$MERGED" | cat

@@ -264,7 +289,8 @@ run_merge_tool () {
 		fi
 		;;
 	p4merge)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
 			$base_present || >"$BASE"
 			"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"

@@ -274,8 +300,10 @@ run_merge_tool () {
 		fi
 		;;
 	tkdiff)
-		if merge_mode; then
-			if $base_present; then
+		if merge_mode
+		then
+			if $base_present
+			then
 				"$merge_tool_path" -a "$BASE" \
 					-o "$MERGED" "$LOCAL" "$REMOTE"
 			else

@@ -288,7 +316,8 @@ run_merge_tool () {
 		fi
 		;;
 	tortoisemerge)
-		if $base_present; then
+		if $base_present
+		then
 			touch "$BACKUP"
 			"$merge_tool_path" \
 				-base:"$BASE" -mine:"$LOCAL" \

@@ -300,9 +329,11 @@ run_merge_tool () {
 		fi
 		;;
 	xxdiff)
-		if merge_mode; then
+		if merge_mode
+		then
 			touch "$BACKUP"
-			if $base_present; then
+			if $base_present
+			then
 				"$merge_tool_path" -X --show-merged-pane \
 					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
 					-R 'Accel.Search: "Ctrl+F"' \

@@ -327,16 +358,20 @@ run_merge_tool () {
 		;;
 	*)
 		merge_tool_cmd="$(get_merge_tool_cmd "$1")"
-		if test -z "$merge_tool_cmd"; then
-			if merge_mode; then
+		if test -z "$merge_tool_cmd"
+		then
+			if merge_mode
+			then
 				status=1
 			fi
 			break
 		fi
-		if merge_mode; then
+		if merge_mode
+		then
 			trust_exit_code="$(git config --bool \
 				mergetool."$1".trustExitCode || echo false)"
-			if test "$trust_exit_code" = "false"; then
+			if test "$trust_exit_code" = "false"
+			then
 				touch "$BACKUP"
 				( eval $merge_tool_cmd )
 				check_unchanged

@@ -353,13 +388,16 @@ run_merge_tool () {
 }
 
 guess_merge_tool () {
-	if merge_mode; then
+	if merge_mode
+	then
 		tools="tortoisemerge"
 	else
 		tools="kompare"
 	fi
-	if test -n "$DISPLAY"; then
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+	if test -n "$DISPLAY"
+	then
+		if test -n "$GNOME_DESKTOP_SESSION_ID"
+		then
 			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
 		else
 			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"

@@ -380,7 +418,8 @@ guess_merge_tool () {
 	for i in $tools
 	do
 		merge_tool_path="$(translate_merge_tool_path "$i")"
-		if type "$merge_tool_path" > /dev/null 2>&1; then
+		if type "$merge_tool_path" >/dev/null 2>&1
+		then
 			echo "$i"
 			return 0
 		fi

@@ -393,12 +432,14 @@ guess_merge_tool () {
 get_configured_merge_tool () {
 	# Diff mode first tries diff.tool and falls back to merge.tool.
 	# Merge mode only checks merge.tool
-	if diff_mode; then
+	if diff_mode
+	then
 		merge_tool=$(git config diff.tool || git config merge.tool)
 	else
 		merge_tool=$(git config merge.tool)
 	fi
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"
+	then
 		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
 		echo >&2 "Resetting to default..."
 		return 1

@@ -408,26 +449,31 @@ get_configured_merge_tool () {
 
 get_merge_tool_path () {
 	# A merge tool has been set, so verify that it's valid.
-	if test -n "$1"; then
+	if test -n "$1"
+	then
 		merge_tool="$1"
 	else
 		merge_tool="$(get_merge_tool)"
 	fi
-	if ! valid_tool "$merge_tool"; then
+	if ! valid_tool "$merge_tool"
+	then
 		echo >&2 "Unknown merge tool $merge_tool"
 		exit 1
 	fi
-	if diff_mode; then
+	if diff_mode
+	then
 		merge_tool_path=$(git config difftool."$merge_tool".path ||
 		                  git config mergetool."$merge_tool".path)
 	else
 		merge_tool_path=$(git config mergetool."$merge_tool".path)
 	fi
-	if test -z "$merge_tool_path"; then
+	if test -z "$merge_tool_path"
+	then
 		merge_tool_path="$(translate_merge_tool_path "$merge_tool")"
 	fi
 	if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
-	! type "$merge_tool_path" > /dev/null 2>&1; then
+		! type "$merge_tool_path" >/dev/null 2>&1
+	then
 		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as"\
 		         "'$merge_tool_path'"
 		exit 1

@@ -439,7 +485,8 @@ get_merge_tool () {
 	# Check if a merge tool has been configured
 	merge_tool=$(get_configured_merge_tool)
 	# Try to guess an appropriate merge tool if no tool has been set.
-	if test -z "$merge_tool"; then
+	if test -z "$merge_tool"
+	then
 		merge_tool="$(guess_merge_tool)" || exit
 	fi
 	echo "$merge_tool"

Signature · 1 line

1.7.6.476.g57292

David Aguilar

author

2016-06-15 22:51 UTC #6 lore raw

PATCH 3/4 mergetool--lib: Refactor tools into separate files

In reply to David Aguilar

Individual merge tools are now defined in a mergetools/$tool file which is sourced at runtime.

The individual files are installed into $(git --exec-path)/mergetools/. New tools can be added by creating a new file instead of editing the git-mergetool--lib.sh scriptlet.

http://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006

Signed-off-by David Aguilar <[email protected]>

---
 Makefile                 |   11 ++
 git-mergetool--lib.sh    |  385 ++++++---------------------------------------
 mergetools/araxis        |   21 +++
 mergetools/bc3           |   20 +++
 mergetools/defaults      |   46 ++++++
 mergetools/diffuse       |   17 ++
 mergetools/ecmerge       |   16 ++
 mergetools/emerge        |   23 +++
 mergetools/kdiff3        |   24 +++
 mergetools/kompare       |    7 +
 mergetools/meld          |    9 +
 mergetools/opendiff      |   16 ++
 mergetools/p4merge       |   10 ++
 mergetools/tkdiff        |   12 ++
 mergetools/tortoisemerge |   17 ++
 mergetools/vim           |   44 ++++++
 mergetools/xxdiff        |   25 +++
 17 files changed, 370 insertions(+), 333 deletions(-)
 create mode 100644 mergetools/araxis
 create mode 100644 mergetools/bc3
 create mode 100644 mergetools/defaults
 create mode 100644 mergetools/diffuse
 create mode 100644 mergetools/ecmerge
 create mode 100644 mergetools/emerge
 create mode 100644 mergetools/kdiff3
 create mode 100644 mergetools/kompare
 create mode 100644 mergetools/meld
 create mode 100644 mergetools/opendiff
 create mode 100644 mergetools/p4merge
 create mode 100644 mergetools/tkdiff
 create mode 100644 mergetools/tortoisemerge
 create mode 100644 mergetools/vim
 create mode 100644 mergetools/xxdiff

Makefile +11 -0

index 8dd782f..6a68e3a 100644
@@ -302,6 +302,7 @@ bindir = $(prefix)/$(bindir_relative)
 mandir = share/man
 infodir = share/info
 gitexecdir = libexec/git-core
+mergetoolsdir = $(gitexecdir)/mergetools
 sharedir = $(prefix)/share
 gitwebdir = $(sharedir)/gitweb
 template_dir = share/git-core/templates

@@ -2257,6 +2258,13 @@ endif
 gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
 export gitexec_instdir
 
+ifneq ($(filter /%,$(firstword $(mergetoolsdir))),)
+mergetools_instdir = $(mergetoolsdir)
+else
+mergetools_instdir = $(prefix)/$(mergetoolsdir)
+endif
+mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir))
+
 install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
 
 install: all

@@ -2266,6 +2274,9 @@ install: all
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+	(cd mergetools && $(TAR) cf - .) | \
+	(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C gitweb install

git-mergetool--lib.sh +52 ~~-333~~

431 lines not on this page · raw message

mergetools/araxis new file +21 -0

new file mode 100644
index 0000000..69a9c8c
@@ -0,0 +1,21 @@
+diff_cmd () {
+	"$merge_tool_path" -wait -2 "$LOCAL" "$REMOTE" >/dev/null 2>&1
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" -wait -merge -3 -a1 \
+			"$BASE" "$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1
+	else
+		"$merge_tool_path" -wait -2 \
+			"$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1
+	fi
+	check_unchanged
+}
+
+translate_merge_tool_path() {
+	echo compare
+}
+

mergetools/bc3 new file +20 -0

new file mode 100644
index 0000000..27b3dd4
@@ -0,0 +1,20 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
+			-mergeoutput="$MERGED"
+	else
+		"$merge_tool_path" "$LOCAL" "$REMOTE" \
+			-mergeoutput="$MERGED"
+	fi
+	check_unchanged
+}
+
+translate_merge_tool_path() {
+	echo bcompare
+}

mergetools/defaults new file +46 -0

46 lines not on this page · raw message

mergetools/diffuse new file +17 -0

new file mode 100644
index 0000000..02e0843
@@ -0,0 +1,17 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" \
+			"$LOCAL" "$MERGED" "$REMOTE" \
+			"$BASE" | cat
+	else
+		"$merge_tool_path" \
+			"$LOCAL" "$MERGED" "$REMOTE" | cat
+	fi
+	check_unchanged
+}

mergetools/ecmerge new file +16 -0

new file mode 100644
index 0000000..13c2e43
@@ -0,0 +1,16 @@
+diff_cmd () {
+	"$merge_tool_path" --default --mode=diff2 "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+			--default --mode=merge3 --to="$MERGED"
+	else
+		"$merge_tool_path" "$LOCAL" "$REMOTE" \
+			--default --mode=merge2 --to="$MERGED"
+	fi
+	check_unchanged
+}

mergetools/emerge new file +23 -0

new file mode 100644
index 0000000..f96d9e5
@@ -0,0 +1,23 @@
+diff_cmd () {
+	"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	if $base_present
+	then
+		"$merge_tool_path" \
+			-f emerge-files-with-ancestor-command \
+			"$LOCAL" "$REMOTE" "$BASE" \
+			"$(basename "$MERGED")"
+	else
+		"$merge_tool_path" \
+			-f emerge-files-command \
+			"$LOCAL" "$REMOTE" \
+			"$(basename "$MERGED")"
+	fi
+	status=$?
+}
+
+translate_merge_tool_path() {
+	echo emacs
+}

mergetools/kdiff3 new file +24 -0

new file mode 100644
index 0000000..28fead4
@@ -0,0 +1,24 @@
+diff_cmd () {
+	"$merge_tool_path" --auto \
+		--L1 "$MERGED (A)" --L2 "$MERGED (B)" \
+		"$LOCAL" "$REMOTE" >/dev/null 2>&1
+}
+
+merge_cmd () {
+	if $base_present
+	then
+		"$merge_tool_path" --auto \
+			--L1 "$MERGED (Base)" \
+			--L2 "$MERGED (Local)" \
+			--L3 "$MERGED (Remote)" \
+			-o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" \
+		>/dev/null 2>&1
+	else
+		"$merge_tool_path" --auto \
+			--L1 "$MERGED (Local)" \
+			--L2 "$MERGED (Remote)" \
+			-o "$MERGED" "$LOCAL" "$REMOTE" \
+		>/dev/null 2>&1
+	fi
+	status=$?
+}

mergetools/kompare new file +7 -0

new file mode 100644
index 0000000..433686c
@@ -0,0 +1,7 @@
+can_merge () {
+	return 1
+}
+
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE"
+}

mergetools/meld new file +9 -0

new file mode 100644
index 0000000..73d70ae
@@ -0,0 +1,9 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	check_unchanged
+}

mergetools/opendiff new file +16 -0

new file mode 100644
index 0000000..0942b2a
@@ -0,0 +1,16 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" "$LOCAL" "$REMOTE" \
+			-ancestor "$BASE" -merge "$MERGED" | cat
+	else
+		"$merge_tool_path" "$LOCAL" "$REMOTE" \
+			-merge "$MERGED" | cat
+	fi
+	check_unchanged
+}

mergetools/p4merge new file +10 -0

new file mode 100644
index 0000000..1a45c1b
@@ -0,0 +1,10 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	$base_present || >"$BASE"
+	"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
+	check_unchanged
+}

mergetools/tkdiff new file +12 -0

new file mode 100644
index 0000000..618c438
@@ -0,0 +1,12 @@
+diff_cmd () {
+	"$merge_tool_path" "$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	if $base_present
+	then
+		"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
+	else
+		"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
+	fi
+}

mergetools/tortoisemerge new file +17 -0

new file mode 100644
index 0000000..ed7db49
@@ -0,0 +1,17 @@
+can_diff () {
+	return 1
+}
+
+merge_cmd () {
+	if $base_present
+	then
+		touch "$BACKUP"
+		"$merge_tool_path" \
+			-base:"$BASE" -mine:"$LOCAL" \
+			-theirs:"$REMOTE" -merged:"$MERGED"
+		check_unchanged
+	else
+		echo "TortoiseMerge cannot be used without a base" 1>&2
+		return 1
+	fi
+}

mergetools/vim new file +44 -0

44 lines not on this page · raw message

mergetools/xxdiff new file +25 -0

new file mode 100644
index 0000000..05b4433
@@ -0,0 +1,25 @@
+diff_cmd () {
+	"$merge_tool_path" \
+		-R 'Accel.Search: "Ctrl+F"' \
+		-R 'Accel.SearchForward: "Ctrl-G"' \
+		"$LOCAL" "$REMOTE"
+}
+
+merge_cmd () {
+	touch "$BACKUP"
+	if $base_present
+	then
+		"$merge_tool_path" -X --show-merged-pane \
+			-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+			-R 'Accel.Search: "Ctrl+F"' \
+			-R 'Accel.SearchForward: "Ctrl-G"' \
+			--merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
+	else
+		"$merge_tool_path" -X $extra \
+			-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+			-R 'Accel.Search: "Ctrl+F"' \
+			-R 'Accel.SearchForward: "Ctrl-G"' \
+			--merged-file "$MERGED" "$LOCAL" "$REMOTE"
+	fi
+	check_unchanged
+}

Signature · 1 line

1.7.6.476.g57292

Jonathan Nieder

2016-06-15 22:52 UTC #7 lore raw

PATCH 1/1 Makefile: fix permissions of mergetools/ checked out with permissive umask

Ever since mergetool--lib was split into multiple files in v1.7.7-rc0~3^2~1 (2011-08-18), the Makefile takes care to reset umask and use tar --no-owner when installing merge tool definitions to $(gitexecdir)/mergetools/. Unfortunately it does not take into account the possibility that the permission bits of the files being copied might already be wrong.

Rather than fixing the "tar" incantation and making it even more complicated, let's just use the "install" utility. This only means losing the ability to install executables and subdirectories of mergetools/, which wasn't used.

Noticed by installing from a copy of git checked out with umask 002. Compare v1.6.0.3~81^2 (Fix permission bits on sources checked out with an overtight umask, 2008-08-21).

Signed-off-by Jonathan Nieder <[email protected]>

---

Quoted 1 line · David Aguilar

+++ b/Makefile

[...]

Review install: all · line 2279

Quoted 7 lines

(quoted diff) +3 -0

@@ -2266,6 +2274,9 @@ install: all
 	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+	(cd mergetools && $(TAR) cf - .) | \
+	(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)

Last month I tried this out and found that, strangely, my files under /usr/lib/git-core/mergetools/ had the g+w bit set. Leading me to
wonder: does the "umask" here have any effect at all?

Since debian/rules install is run as root, the default is for tar to act as thought --preserve-permissions were passed, so the umask when running "tar" is not relevant. Luckily I think "tar" is overkill here, anyway.

Thoughts? Sorry to have taken so long to send this out.

 Makefile |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Makefile +1 -2

index 1e91b19c..e27755e7 100644
@@ -2275,8 +2275,7 @@ install: all
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
-	(cd mergetools && $(TAR) cf - .) | \
-	(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)
+	$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C gitweb install

Signature · 1 line

1.7.7.rc1

Jonathan Nieder

2016-06-15 22:52 UTC #8 lore raw

Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask

Quoted 1 line · Jonathan Nieder

---

[...]

Quoted 2 lines

Since debian/rules install is run as root, the default is for tar to act as thought --preserve-permissions were passed

I should have said: "when 'make install' is run as root, ...".

Typically people building git for private use would run "make install" as root when installing to /usr/local, but as an unprivileged user when installing to $HOME. The RPM packaging runs "make install" without special privileges and the Debian packaging runs it as (fake) root, iirc.

Sorry for the lack of clarity.

David Aguilar

author

2016-06-15 22:52 UTC #9 lore raw

Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask

Quoted 9 lines · Jonathan Nieder

Quoted 1 line · Jonathan Nieder
---
[...]
Quoted 2 lines
Since debian/rules install is run as root, the default is for tar to act as thought --preserve-permissions were passed
I should have said: "when 'make install' is run as root, ...".

Quoted 7 lines

Typically people building git for private use would run "make install" as root when installing to /usr/local, but as an unprivileged user when installing to $HOME. The RPM packaging runs "make install" without special privileges and the Debian packaging runs it as (fake) root, iirc.
Sorry for the lack of clarity.

thanks. I agree that the tar is overkill. I think I copied that snippet from templates/makefile. does that have the same bug?

Signature · 1 line

    David (mobile)

Jonathan Nieder

2016-06-15 22:52 UTC #10 lore raw

Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask

Quoted 2 lines · David Aguilar

thanks. I agree that the tar is overkill. I think I copied that snippet from templates/makefile. does that have the same bug?

It did have a similar bug before, but it was fixed (in a different way) by v1.6.0.3~81^2 (Fix permission bits on sources checked out with an overtight umask, 2008-08-21). The main difference from the mergetools/ case is that the blt/ directory is populated at build time.

Junio C Hamano

2016-06-15 22:52 UTC #11 lore raw

Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask

In reply to David Aguilar

Quoted 2 lines · David Aguilar

thanks. I agree that the tar is overkill. I think I copied that snippet from templates/makefile. does that have the same bug?

Likely, as I recall that I wrote the installation of templates in an expecially sloppy way, thinking somebody would fix it in short order anyway. Instead the sloppiness seems to have been copied and pasted to make things worse...

Thanks for fixing the mess---all's well that ends well.

David Aguilar

author

2016-06-15 22:51 UTC #12 lore raw

PATCH 4/4 mergetools/meld: Use '--output' when available

In reply to David Aguilar

meld 1.5.0 and newer allow the output file to be specified when merging multiple files. Check the meld version and use the '--output' option when available.

Signed-off-by David Aguilar <[email protected]>

---
 mergetools/meld |   33 ++++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 1 deletions(-)

mergetools/meld +32 -1

index 73d70ae..1923797 100644
@@ -4,6 +4,37 @@ diff_cmd () {
 
 merge_cmd () {
 	touch "$BACKUP"
-	"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	if test "$meld_has_output_option" = true
+	then
+		"$merge_tool_path" --output "$MERGED" \
+			"$BASE" "$LOCAL" "$REMOTE"
+	else
+		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	fi
 	check_unchanged
 }
+
+check_meld_version () {
+	if test -n "$meld_version_checked"
+	then
+		return
+	fi
+	# Whether 'meld --output <file>' is supported
+	meld_has_output_option=false
+
+	# Filter meld --version to contain numbers and dots only
+	meld="$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\+//g')"
+	meld="${meld:-0.0.0}"
+
+	meld_major="$(expr "$meld" : '\([0-9]\{1,\}\)' || echo 0)"
+	meld_minor="$(expr "$meld" : '[0-9]\{1,\}\.\([0-9]\{1,\}\)' || echo 0)"
+
+	# 'meld --output <file>' was introduced in meld 1.5.0
+	if test "$meld_major" -gt 1 ||
+		(test "$meld_major" = 1 && test "$meld_minor" -ge 5)
+	then
+		meld_has_output_option=true
+	fi
+	meld_version_checked=true
+}
+check_meld_version

Signature · 1 line

1.7.6.476.g57292

Jonathan Nieder

2016-06-15 22:51 UTC #13 lore raw

Re: [PATCH 4/4] mergetools/meld: Use '--output' when available

Quoted 1 line · David Aguilar

use the '--output' option when available.

Yay. :)

mergetools/meld diff_cmd () { · line 10

Quoted 11 lines

mergetools/meld +4 -1

@@ -4,6 +4,37 @@ diff_cmd () {
 
 merge_cmd () {
 	touch "$BACKUP"
-	"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	if test "$meld_has_output_option" = true
+	then
+		"$merge_tool_path" --output "$MERGED" \
+			"$BASE" "$LOCAL" "$REMOTE"

Shouldn't this be "$LOCAL" "$BASE" "$REMOTE"?

mergetools/meld diff_cmd () { · line 14

Quoted 4 lines

mergetools/meld +3 -0

+	else
+		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	fi
 	check_unchanged

I wonder if the version test could be made a little simpler (perhaps to cope better if future versions use a different numbering system):

	if "$merge_tool_path" --output /dev/null --help >/dev/null 2>&1
	then
		"$merge_tool_path" --output ...
	else
		"$merge_tool_path" "$LOCAL" ...
	fi

Forgive my ignorance: is this function likely to be called in a loop? If so, it makes sense to precompute or cache the result of detection, like you already do.

	check_meld_for_output_option () {
		if ...
		then
			meld_has_output_option=true
		else
			meld_has_output_option=false
		fi
	}

	merge_cmd () {
		if test -z "${meld_has_output_option:+set}"
		then
			check_meld_for_output_option
		fi

		if test "$meld_has_output_option" = true
		then
			...

[...]

mergetools/meld diff_cmd () { · line 26

Quoted 2 lines

mergetools/meld +2 -0

+	# Filter meld --version to contain numbers and dots only
+	meld="$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\+//g')"

\+ is not a BRE. If parsing version numbers seems like the right thing to do, maybe "tr -cd 0-9."?

mergetools/meld diff_cmd () { · line 30

Quoted 4 lines

mergetools/meld +4 -0

+	meld="${meld:-0.0.0}"
+
+	meld_major="$(expr "$meld" : '\([0-9]\{1,\}\)' || echo 0)"
+	meld_minor="$(expr "$meld" : '[0-9]\{1,\}\.\([0-9]\{1,\}\)' || echo 0)"

I think git avoids \{m,n\} ranges where possible (for portability). This could be:

	meld_major=${meld%%.*}
	meld_nonmajor=${meld#${meld_major}.}
	meld_minor=${meld_nonmajor%%.*}

or:

	case $meld in
	[2-9].* | [1-9][0-9]* | 1.[5-9]* | 1.[1-9][0-9]*)	# >= 1.5.0
		meld_has_output_option=true ;;
	*)
		meld_has_output_option=false ;;
	esac

It's nice how self-contained this can be now that it's in its own file. Thanks.

David Aguilar

author

2016-06-15 22:51 UTC #14 lore raw

Re: [PATCH 4/4] mergetools/meld: Use '--output' when available

Quoted 10 lines · Jonathan Nieder

Quoted 1 line · David Aguilar
use the '--output' option when available.
Yay. :)
Quoted 4 lines
mergetools/meld +0 -0
@@ -4,6 +4,37 @@ diff_cmd () {
 

Quoted 9 lines

mergetools/meld diff_cmd () { · line 10
Quoted 7 lines
mergetools/meld +4 -1
 merge_cmd () {
 	touch "$BACKUP"
-	"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	if test "$meld_has_output_option" = true
+	then
+		"$merge_tool_path" --output "$MERGED" \
+			"$BASE" "$LOCAL" "$REMOTE"
Shouldn't this be "$LOCAL" "$BASE" "$REMOTE"?

Yup, thanks.

Quoted 16 lines

mergetools/meld diff_cmd () { · line 14
Quoted 5 lines
mergetools/meld +3 -0
+	else
+		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	fi
 	check_unchanged
I wonder if the version test could be made a little simpler (perhaps to cope better if future versions use a different numbering system):
	if "$merge_tool_path" --output /dev/null --help >/dev/null 2>&1
	then
		"$merge_tool_path" --output ...
	else
		"$merge_tool_path" "$LOCAL" ...
	fi

Quoted 3 lines

Forgive my ignorance: is this function likely to be called in a loop? If so, it makes sense to precompute or cache the result of detection, like you already do.

Yes, it is called in a loop...

Quoted 48 lines

	check_meld_for_output_option () {
		if ...
		then
			meld_has_output_option=true
		else
			meld_has_output_option=false
		fi
	}

	merge_cmd () {
		if test -z "${meld_has_output_option:+set}"
		then
			check_meld_for_output_option
		fi

		if test "$meld_has_output_option" = true
		then
			...

[...]

mergetools/meld diff_cmd () { · line 26

Quoted 2 lines

mergetools/meld +2 -0

+	# Filter meld --version to contain numbers and dots only
+	meld="$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\+//g')"

\+ is not a BRE. If parsing version numbers seems like the right thing to do, maybe "tr -cd 0-9."?

mergetools/meld diff_cmd () { · line 30

Quoted 4 lines

mergetools/meld +4 -0

+	meld="${meld:-0.0.0}"
+
+	meld_major="$(expr "$meld" : '\([0-9]\{1,\}\)' || echo 0)"
+	meld_minor="$(expr "$meld" : '[0-9]\{1,\}\.\([0-9]\{1,\}\)' || echo 0)"

I think git avoids \{m,n\} ranges where possible (for portability). This could be:

	meld_major=${meld%%.*}
	meld_nonmajor=${meld#${meld_major}.}
	meld_minor=${meld_nonmajor%%.*}

or:

	case $meld in
	[2-9].* | [1-9][0-9]* | 1.[5-9]* | 1.[1-9][0-9]*)	# >= 1.5.0
		meld_has_output_option=true ;;
	*)
		meld_has_output_option=false ;;
	esac

Quoted 2 lines

It's nice how self-contained this can be now that it's in its own file. Thanks.

Right, I was using \{1,\} since that's what CodingStyle said to use instead of \+ (which I forgot to fixup above as you saw).

The case statement is nice and simple enough to understand. By doing meld --output /dev/null --help we're relying on older versions blowing up with --output and --help returning exit status 0. That seems pretty reasonable.
The case statement does seem sufficient but just trying --output and seeing what happens is even simpler. Your example also uses $merge_tool_path, which I forgot to do, so I'll be sure to include that too.

I'll wait until tomorrow to see if there are any more comments and reroll.

Thanks,

Signature · 1 line

					David

David Aguilar

author

2016-06-15 22:51 UTC #15 lore raw

PATCH v2 4/4 mergetools/meld: Use '--output' when available

In reply to David Aguilar

meld 1.5.0 and newer allow the output file to be specified when merging multiple files. Check whether the meld command supports '--output' and use it when available.

Signed-off-by David Aguilar <[email protected]>

---
 mergetools/meld |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

Changes since v1 with help from Jonathan Nieder:

Use $LOCAL $BASE REMOTE when calling meld.
Avoid shell portability issues by using the return status. of --output instead of parsing --version.

mergetools/meld +24 -1

index 73d70ae..eaa115c 100644
@@ -3,7 +3,30 @@ diff_cmd () {
 }
 
 merge_cmd () {
+	if test -z "${meld_has_output_option:+set}"
+	then
+		check_meld_for_output_version
+	fi
 	touch "$BACKUP"
-	"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	if test "$meld_has_output_option" = true
+	then
+		"$merge_tool_path" --output "$MERGED" \
+			"$LOCAL" "$BASE" "$REMOTE"
+	else
+		"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+	fi
 	check_unchanged
 }
+
+# Check whether 'meld --output <file>' is supported
+check_meld_for_output_version () {
+	meld_path="$(git config mergetool.meld.path)"
+	meld_path="${meld_path:-meld}"
+
+	if "$meld_path" --output /dev/null --help >/dev/null 2>&1
+	then
+		meld_has_output_option=true
+	else
+		meld_has_output_option=false
+	fi
+}

Signature · 1 line

1.7.6.553.g1899

Junio C Hamano

2016-06-15 22:51 UTC #16 lore raw

Re: [PATCH v2 4/4] mergetools/meld: Use '--output' when available

Quoted 10 lines · David Aguilar

meld 1.5.0 and newer allow the output file to be specified when merging multiple files. Check whether the meld command supports '--output' and use it when available.
Signed-off-by David Aguilar <[email protected]>
---
 mergetools/meld |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)
Changes since v1 with help from Jonathan Nieder:

Thanks, both of you. Queued all patches in the series.

Sebastian Schuberth

2016-06-15 22:51 UTC #17 lore raw

In reply to David Aguilar

Quoted 2 lines · David Aguilar

This series splits the git-mergetool--lib.sh file into separate tool scriptlets that define the commands for diff and merge.

Very nice as it simplifies adding new merge tools to adding a single file with little code inside. Thanks!

Signature · 1 line

Sebastian Schuberth