* [PATCH v5 1/3] vimdiff: new implementation with layout support
2022-03-19 9:11 [PATCH v5 0/3] vimdiff: new implementation with layout support Fernando Ramos
@ 2022-03-19 9:11 ` Fernando Ramos
2022-03-23 16:43 ` Junio C Hamano
2022-03-19 9:11 ` [PATCH v5 2/3] vimdiff: add tool documentation Fernando Ramos
2022-03-19 9:11 ` [PATCH v5 3/3] vimdiff: integrate layout tests in the unit tests framework ('t' folder) Fernando Ramos
2 siblings, 1 reply; 8+ messages in thread
From: Fernando Ramos @ 2022-03-19 9:11 UTC (permalink / raw)
To: git; +Cc: gitster, davvid, sunshine, seth, levraiphilippeblain, rogi
When running 'git mergetool -t vimdiff', a new configuration option
('mergetool.vimdiff.layout') can now be used to select how the user
wants the different windows, tabs and buffers to be displayed.
If the option is not provided, the layout will be the same one that was
being used before this commit (ie. two rows with LOCAL, BASE and COMMIT
in the top one and MERGED in the bottom one).
The 'vimdiff' variants ('vimdiff{1,2,3}') still work but, because they
represented nothing else than different layouts, are now internally
implemented as a subcase of 'vimdiff' with the corresponding
pre-configured 'layout'.
Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
mergetools/vimdiff | 548 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 521 insertions(+), 27 deletions(-)
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 96f6209a04..5bf77a5388 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -1,48 +1,440 @@
+# This script can be run in two different contexts:
+#
+# - From git, when the user invokes the "vimdiff" merge tool. In this context
+# this script expects the following environment variables (among others) to
+# be defined (which is something "git" takes care of):
+#
+# - $BASE
+# - $LOCAL
+# - $REMOTE
+# - $MERGED
+#
+# In this mode, all this script does is to run the next command:
+#
+# vim -f -c ... $LOCAL $BASE $REMOTE $MERGED
+#
+# ...where the "..." string depends on the value of the
+# "mergetool.vimdiff.layout" configuration variable and is used to open vim
+# with a certain layout of buffers, windows and tabs.
+#
+# - From the unit tests framework ("t" folder) by sourcing this script and
+# then manually calling "run_unit_tests", which will run a battery of unit
+# tests to make sure nothing breaks.
+# In this context this script does not expect any particular environment
+# variable to be set.
+
+
+################################################################################
+## Internal functions (not meant to be used outside this script)
+################################################################################
+
+debug_print() {
+ # Send message to stderr if global variable DEBUG is set to "true"
+
+ if test -n "$GIT_MERGETOOL_VIMDIFF_DEBUG"
+ then
+ >&2 echo "$@"
+ fi
+}
+
+substring() {
+ # Return a substring of $1 containing $3 characters starting at
+ # zero-based offset $2.
+ #
+ # Examples:
+ #
+ # substring "Hello world" 0 4 --> "Hell"
+ # substring "Hello world" 3 4 --> "lo w"
+ # substring "Hello world" 3 10 --> "lo world"
+
+ STRING=$1
+ START=$2
+ LEN=$3
+
+ echo "$STRING" | cut -c$(( START + 1 ))-$(( START + $LEN))
+}
+
+gen_cmd_aux() {
+ # Auxiliary function used from "gen_cmd()".
+ # Read that other function documentation for more details.
+
+ LAYOUT=$1
+ CMD=$2 # This is a second (hidden) argument used for recursion
+
+ debug_print
+ debug_print "LAYOUT : $LAYOUT"
+ debug_print "CMD : $CMD"
+
+ if test -z "$CMD"
+ then
+ CMD="echo" # vim "nop" operator
+ fi
+
+ start=0
+ end=${#LAYOUT}
+
+ nested=0
+ nested_min=100
+
+
+ # Step 1:
+ #
+ # Increase/decrease "start"/"end" indices respectively to get rid of
+ # outer parenthesis.
+ #
+ # Example:
+ #
+ # - BEFORE: (( LOCAL , BASE ) / MERGED )
+ # - AFTER : ( LOCAL , BASE ) / MERGED
+
+ oldIFS=$IFS
+ IFS=#
+ for c in $(echo "$LAYOUT" | sed 's:.:&#:g')
+ do
+ if test "$c" = " "
+ then
+ continue
+ fi
+
+ if test "$c" = "("
+ then
+ nested=$(( nested + 1 ))
+ continue
+ fi
+
+ if test "$c" = ")"
+ then
+ nested=$(( nested - 1 ))
+ continue
+ fi
+
+ if test "$nested" -lt "$nested_min"
+ then
+ nested_min=$nested
+ fi
+ done
+ IFS=$oldIFS
+
+ debug_print "NESTED MIN: $nested_min"
+
+ while test "$nested_min" -gt "0"
+ do
+ start=$(( start + 1 ))
+ end=$(( end - 1 ))
+
+ start_minus_one=$(( start - 1 ))
+
+ while ! test "$(substring "$LAYOUT" "$start_minus_one" 1)" = "("
+ do
+ start=$(( start + 1 ))
+ start_minus_one=$(( start_minus_one + 1 ))
+ done
+
+ while ! test "$(substring "$LAYOUT" "$end" 1)" = ")"
+ do
+ end=$(( end - 1 ))
+ done
+
+ nested_min=$(( nested_min - 1 ))
+ done
+
+ debug_print "CLEAN : $(substring "$LAYOUT" "$start" "$(( end - start ))")"
+
+
+ # Step 2:
+ #
+ # Search for all valid separators ("+", "/" or ",") which are *not*
+ # inside parenthesis. Save the index at which each of them makes the
+ # first appearance.
+
+ index_new_tab=""
+ index_horizontal_split=""
+ index_vertical_split=""
+
+ nested=0
+ i=$(( start - 1 ))
+
+ oldIFS=$IFS
+ IFS=#
+ for c in $(substring "$LAYOUT" "$start" "$(( end - start ))" | sed 's:.:&#:g');
+ do
+ i=$(( i + 1 ))
+
+ if test "$c" = " "
+ then
+ continue
+ fi
+
+ if test "$c" = "("
+ then
+ nested=$(( nested + 1 ))
+ continue
+ fi
+
+ if test "$c" = ")"
+ then
+ nested=$(( nested - 1 ))
+ continue
+ fi
+
+ if test "$nested" = 0
+ then
+ current=$c
+
+ if test "$current" = "+"
+ then
+ if test -z "$index_new_tab"
+ then
+ index_new_tab=$i
+ fi
+
+ elif test "$current" = "/"
+ then
+ if test -z "$index_horizontal_split"
+ then
+ index_horizontal_split=$i
+ fi
+
+ elif test "$current" = ","
+ then
+ if test -z "$index_vertical_split"
+ then
+ index_vertical_split=$i
+ fi
+ fi
+ fi
+ done
+ IFS=$oldIFS
+
+
+ # Step 3:
+ #
+ # Process the separator with the highest order of precedence
+ # (";" has the highest precedence and "|" the lowest one).
+ #
+ # By "process" I mean recursively call this function twice: the first
+ # one with the substring at the left of the separator and the second one
+ # with the one at its right.
+
+ terminate="false"
+
+ if ! test -z "$index_new_tab"
+ then
+ before="-tabnew"
+ after="tabnext"
+ index=$index_new_tab
+ terminate="true"
+
+ elif ! test -z "$index_horizontal_split"
+ then
+ before="split"
+ after="wincmd j"
+ index=$index_horizontal_split
+ terminate="true"
+
+ elif ! test -z "$index_vertical_split"
+ then
+ before="vertical split"
+ after="wincmd l"
+ index=$index_vertical_split
+ terminate="true"
+ fi
+
+ if test "$terminate" = "true"
+ then
+ CMD="$CMD | $before"
+ CMD=$(gen_cmd_aux "$(substring "$LAYOUT" "$start" "$(( index - start ))")" "$CMD")
+ CMD="$CMD | $after"
+ CMD=$(gen_cmd_aux "$(substring "$LAYOUT" "$(( index + 1 ))" "$(( ${#LAYOUT} - index ))")" "$CMD")
+ echo "$CMD"
+ return
+ fi
+
+
+ # Step 4:
+ #
+ # If we reach this point, it means there are no separators and we just
+ # need to print the command to display the specified buffer
+
+ target=$(substring "$LAYOUT" "$start" "$(( end - start ))" | sed 's:[ @();|-]::g')
+
+ if test "$target" = "LOCAL"
+ then
+ CMD="$CMD | 1b"
+
+ elif test "$target" = "BASE"
+ then
+ CMD="$CMD | 2b"
+
+ elif test "$target" = "REMOTE"
+ then
+ CMD="$CMD | 3b"
+
+ elif test "$target" = "MERGED"
+ then
+ CMD="$CMD | 4b"
+
+ else
+ CMD="$CMD | ERROR: >$target<"
+ fi
+
+ echo "$CMD"
+ return
+}
+
+
+gen_cmd() {
+ # This function returns (in global variable FINAL_CMD) the string that
+ # you can use when invoking "vim" (as shown next) to obtain a given
+ # layout:
+ #
+ # $ vim -f $FINAL_CMD "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
+ #
+ # It takes one single argument: a string containing the desired layout
+ # definition.
+ #
+ # The syntax of the "layout definitions" is explained in ... (TODO)...
+ # but you can already intuitively understand how it works by knowing
+ # that...
+ #
+ # * ";" means "a new vim tab"
+ # * "-" means "a new vim horizontal split"
+ # * "|" means "a new vim vertical split"
+ #
+ # It also returns (in global variable FINAL_TARGET) the name ("LOCAL",
+ # "BASE", "REMOTE" or "MERGED") of the file that is marked with an "@",
+ # or "MERGED" if none of them is.
+ #
+ # Example:
+ #
+ # gen_cmd "@LOCAL , REMOTE"
+ # |
+ # `-> FINAL_CMD == "-c \"echo | vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ # FINAL_TARGET == "LOCAL"
+
+ LAYOUT=$1
+
+
+ # Search for a "@" in one of the files identifiers ("LOCAL", "BASE",
+ # "REMOTE", "MERGED"). If not found, use "MERGE" as the default file
+ # where changes will be saved.
+
+ if echo "$LAYOUT" | grep @LOCAL >/dev/null
+ then
+ FINAL_TARGET="LOCAL"
+ elif echo "$LAYOUT" | grep @BASE >/dev/null
+ then
+ FINAL_TARGET="BASE"
+ else
+ FINAL_TARGET="MERGED"
+ fi
+
+
+ # Obtain the first part of vim "-c" option to obtain the desired layout
+
+ CMD=$(gen_cmd_aux "$LAYOUT")
+
+
+ # Adjust the just obtained script depending on whether more than one
+ # windows are visible or not
+
+ if echo "$LAYOUT" | grep ",\|/" >/dev/null
+ then
+ CMD="$CMD | tabdo windo diffthis"
+ else
+ CMD="$CMD | bufdo diffthis"
+ fi
+
+
+ # Add an extra "-c" option to move to the first tab (notice that we
+ # can't simply append the command to the previous "-c" string as
+ # explained here: https://github.com/vim/vim/issues/9076
+
+ FINAL_CMD="-c \"$CMD\" -c \"tabfirst\""
+}
+
+
+################################################################################
+## API functions (called from "git-mergetool--lib.sh")
+################################################################################
+
diff_cmd () {
"$merge_tool_path" -R -f -d \
-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
}
+
merge_cmd () {
+ layout=$(git config mergetool.$merge_tool.layout)
+
case "$1" in
*vimdiff)
- if $base_present
+ if test -z "$layout"
then
- "$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
- "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
- else
- "$merge_tool_path" -f -d -c 'wincmd l' \
- "$LOCAL" "$MERGED" "$REMOTE"
+ # Default layout when none is specified
+ layout="(LOCAL,BASE,REMOTE)/MERGED"
fi
;;
*vimdiff1)
- "$merge_tool_path" -f -d \
- -c 'echon "Resolve conflicts leftward then save. Use :cq to abort."' \
- "$LOCAL" "$REMOTE"
- ret="$?"
- if test "$ret" -eq 0
- then
- cp -- "$LOCAL" "$MERGED"
- fi
- return "$ret"
+ layout="@LOCAL,MERGED"
;;
*vimdiff2)
- "$merge_tool_path" -f -d -c 'wincmd l' \
- "$LOCAL" "$MERGED" "$REMOTE"
+ layout="LOCAL,MERGED,REMOTE"
;;
*vimdiff3)
- if $base_present
- then
- "$merge_tool_path" -f -d -c 'hid | hid | hid' \
- "$LOCAL" "$REMOTE" "$BASE" "$MERGED"
- else
- "$merge_tool_path" -f -d -c 'hid | hid' \
- "$LOCAL" "$REMOTE" "$MERGED"
- fi
+ layout="MERGED"
;;
esac
+
+ gen_cmd "$layout"
+
+ debug_print ""
+ debug_print "FINAL CMD : $FINAL_CMD"
+ debug_print "FINAL TAR : $FINAL_TARGET"
+
+ if $base_present
+ then
+ eval "$merge_tool_path" \
+ -f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
+ else
+ # If there is no BASE (example: a merge conflict in a new file
+ # with the same name created in both braches which didn't exist
+ # before), close all BASE windows using vim's "quit" command
+
+ FINAL_CMD=$(echo "$FINAL_CMD" | \
+ sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
+
+ eval "$merge_tool_path" \
+ -f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
+ fi
+
+ ret="$?"
+
+ if test "$ret" -eq 0
+ then
+ case "$FINAL_TARGET" in
+ LOCAL)
+ source_path="$LOCAL"
+ ;;
+ REMOTE)
+ source_path="$REMOTE"
+ ;;
+ MERGED|*)
+ # Do nothing
+ source_path=
+ ;;
+ esac
+
+ if test -n "$source_path"
+ then
+ cp "$source_path" "$MERGED"
+ fi
+ fi
+
+ return "$ret"
}
+
translate_merge_tool_path() {
case "$1" in
nvimdiff*)
@@ -57,14 +449,116 @@ translate_merge_tool_path() {
esac
}
+
exit_code_trustable () {
true
}
+
list_tool_variants () {
- for prefix in '' g n; do
- for suffix in '' 1 2 3; do
+ for prefix in '' g n
+ do
+ for suffix in '' 1 2 3
+ do
echo "${prefix}vimdiff${suffix}"
done
done
}
+
+
+################################################################################
+## Unit tests (called from scripts inside the "t" folder)
+################################################################################
+
+run_unit_tests() {
+ # Function to make sure that we don't break anything when modifying this
+ # script.
+ #
+ # This function is automatically executed when you execute this directly
+ # from the shell
+
+ NUMBER_OF_TEST_CASES=16
+
+ TEST_CASE_01="LOCAL,MERGED,REMOTE"
+ TEST_CASE_02="LOCAL/MERGED/REMOTE"
+ TEST_CASE_03="(LOCAL/REMOTE),MERGED"
+ TEST_CASE_04="MERGED,(LOCAL/REMOTE)"
+ TEST_CASE_05="(LOCAL,REMOTE)/MERGED"
+ TEST_CASE_06="MERGED/(LOCAL,REMOTE)"
+ TEST_CASE_07="(LOCAL,BASE,REMOTE)/MERGED"
+ TEST_CASE_08="(LOCAL/BASE/REMOTE),MERGED"
+ TEST_CASE_09="@LOCAL,REMOTE"
+ TEST_CASE_10="MERGED"
+ TEST_CASE_11="(LOCAL,BASE,REMOTE)/MERGED+BASE,LOCAL+BASE,REMOTE+(LOCAL/BASE/REMOTE),MERGED"
+ TEST_CASE_12="((LOCAL,REMOTE)/BASE),MERGED"
+ TEST_CASE_13="((LOCAL,REMOTE)/BASE),((LOCAL/REMOTE),MERGED)"
+ TEST_CASE_14="BASE,REMOTE+BASE,LOCAL"
+ TEST_CASE_15=" (( (LOCAL , BASE , REMOTE) / MERGED)) +(BASE) , LOCAL+ BASE , REMOTE+ (((LOCAL / BASE / REMOTE)) , MERGED ) "
+ TEST_CASE_16="LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL / BASE / REMOTE),MERGED"
+
+ EXPECTED_CMD_01="-c \"echo | vertical split | 1b | wincmd l | vertical split | 4b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_02="-c \"echo | split | 1b | wincmd j | split | 4b | wincmd j | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_03="-c \"echo | vertical split | split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_04="-c \"echo | vertical split | 4b | wincmd l | split | 1b | wincmd j | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_05="-c \"echo | split | vertical split | 1b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_06="-c \"echo | split | 4b | wincmd j | vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_07="-c \"echo | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_08="-c \"echo | vertical split | split | 1b | wincmd j | split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_09="-c \"echo | vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_10="-c \"echo | 4b | bufdo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_11="-c \"echo | -tabnew | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | vertical split | 2b | wincmd l | 3b | tabnext | vertical split | split | 1b | wincmd j | split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_12="-c \"echo | vertical split | split | vertical split | 1b | wincmd l | 3b | wincmd j | 2b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_13="-c \"echo | vertical split | split | vertical split | 1b | wincmd l | 3b | wincmd j | 2b | wincmd l | vertical split | split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_14="-c \"echo | -tabnew | vertical split | 2b | wincmd l | 3b | tabnext | vertical split | 2b | wincmd l | 1b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_15="-c \"echo | -tabnew | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | vertical split | 2b | wincmd l | 3b | tabnext | vertical split | split | 1b | wincmd j | split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+ EXPECTED_CMD_16="-c \"echo | -tabnew | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | vertical split | 2b | wincmd l | 3b | tabnext | vertical split | split | 1b | wincmd j | split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+
+ EXPECTED_TARGET_01="MERGED"
+ EXPECTED_TARGET_02="MERGED"
+ EXPECTED_TARGET_03="MERGED"
+ EXPECTED_TARGET_04="MERGED"
+ EXPECTED_TARGET_05="MERGED"
+ EXPECTED_TARGET_06="MERGED"
+ EXPECTED_TARGET_07="MERGED"
+ EXPECTED_TARGET_08="MERGED"
+ EXPECTED_TARGET_09="LOCAL"
+ EXPECTED_TARGET_10="MERGED"
+ EXPECTED_TARGET_11="MERGED"
+ EXPECTED_TARGET_12="MERGED"
+ EXPECTED_TARGET_13="MERGED"
+ EXPECTED_TARGET_14="MERGED"
+ EXPECTED_TARGET_15="MERGED"
+ EXPECTED_TARGET_16="MERGED"
+
+ at_least_one_ko="false"
+
+ for i in $(seq -w 1 99)
+ do
+ if test "$i" -gt $NUMBER_OF_TEST_CASES
+ then
+ break
+ fi
+
+ gen_cmd "$(eval echo \${TEST_CASE_"$i"})"
+
+ if test "$FINAL_CMD" = "$(eval echo \${EXPECTED_CMD_"$i"})" \
+ && test "$FINAL_TARGET" = "$(eval echo \${EXPECTED_TARGET_"$i"})"
+ then
+ printf "Test Case #%02d: OK\n" "$(echo "$i" | sed 's/^0*//')"
+ else
+ printf "Test Case #%02d: KO !!!!\n" "$(echo "$i" | sed 's/^0*//')"
+ echo " FINAL_CMD : $FINAL_CMD"
+ echo " FINAL_CMD (expected) : $(eval echo \${EXPECTED_CMD_"$i"})"
+ echo " FINAL_TARGET : $FINAL_TARGET"
+ echo " FINAL_TARGET (expected): $(eval echo \${EXPECTED_TARGET_"$i"})"
+ at_least_one_ko="true"
+ fi
+ done
+
+ if test "$at_least_one_ko" = "true"
+ then
+ return 255
+ else
+ return 0
+ fi
+}
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/3] vimdiff: new implementation with layout support
2022-03-19 9:11 ` [PATCH v5 1/3] " Fernando Ramos
@ 2022-03-23 16:43 ` Junio C Hamano
2022-03-24 12:21 ` Fernando
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2022-03-23 16:43 UTC (permalink / raw)
To: Fernando Ramos; +Cc: git, davvid, sunshine, seth, levraiphilippeblain, rogi
Fernando Ramos <greenfoo@u92.eu> writes:
> When running 'git mergetool -t vimdiff', a new configuration option
> ('mergetool.vimdiff.layout') can now be used to select how the user
> wants the different windows, tabs and buffers to be displayed.
>
> If the option is not provided, the layout will be the same one that was
> being used before this commit (ie. two rows with LOCAL, BASE and COMMIT
> in the top one and MERGED in the bottom one).
>
> The 'vimdiff' variants ('vimdiff{1,2,3}') still work but, because they
> represented nothing else than different layouts, are now internally
> implemented as a subcase of 'vimdiff' with the corresponding
> pre-configured 'layout'.
>
> Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
> ---
> mergetools/vimdiff | 548 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 521 insertions(+), 27 deletions(-)
>
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index 96f6209a04..5bf77a5388 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -1,48 +1,440 @@
> +# This script can be run in two different contexts:
> +#
> +# - From git, when the user invokes the "vimdiff" merge tool. In this context
> +# this script expects the following environment variables (among others) to
> +# be defined (which is something "git" takes care of):
> +#
> +# - $BASE
> +# - $LOCAL
> +# - $REMOTE
> +# - $MERGED
> +#
> +# In this mode, all this script does is to run the next command:
> +#
> +# vim -f -c ... $LOCAL $BASE $REMOTE $MERGED
> +#
> +# ...where the "..." string depends on the value of the
> +# "mergetool.vimdiff.layout" configuration variable and is used to open vim
> +# with a certain layout of buffers, windows and tabs.
> +#
> +# - From the unit tests framework ("t" folder) by sourcing this script and
> +# then manually calling "run_unit_tests", which will run a battery of unit
> +# tests to make sure nothing breaks.
> +# In this context this script does not expect any particular environment
> +# variable to be set.
> +
> +
> +################################################################################
> +## Internal functions (not meant to be used outside this script)
> +################################################################################
> +
> +debug_print() {
> + # Send message to stderr if global variable DEBUG is set to "true"
> +
> + if test -n "$GIT_MERGETOOL_VIMDIFF_DEBUG"
> + then
> + >&2 echo "$@"
> + fi
> +}
Do we want to keep this helper, and many calls to it sprinkled in
this file, or are they leftover cruft?
Style. "debug_print () {", i.e. SPACE on both sides of "()".
> +substring() {
> + # Return a substring of $1 containing $3 characters starting at
> + # zero-based offset $2.
> + #
> + # Examples:
> + #
> + # substring "Hello world" 0 4 --> "Hell"
> + # substring "Hello world" 3 4 --> "lo w"
> + # substring "Hello world" 3 10 --> "lo world"
> +
> + STRING=$1
> + START=$2
> + LEN=$3
> +
> + echo "$STRING" | cut -c$(( START + 1 ))-$(( START + $LEN))
> +}
The lack of space before the second closing "))" makes it look
inconsistent. We should be able to do this no external commands
and just two variable substitutions and without relying on
bash-isms, but the above should do.
> merge_cmd () {
> + layout=$(git config mergetool.$merge_tool.layout)
> +
> case "$1" in
> *vimdiff)
> - if $base_present
> + if test -z "$layout"
> then
> - "$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
> - "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
> - else
> - "$merge_tool_path" -f -d -c 'wincmd l' \
> - "$LOCAL" "$MERGED" "$REMOTE"
> + # Default layout when none is specified
> + layout="(LOCAL,BASE,REMOTE)/MERGED"
> fi
> ;;
> ...
> + if $base_present
> + then
> + eval "$merge_tool_path" \
> + -f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
> + else
> + # If there is no BASE (example: a merge conflict in a new file
> + # with the same name created in both braches which didn't exist
> + # before), close all BASE windows using vim's "quit" command
> +
> + FINAL_CMD=$(echo "$FINAL_CMD" | \
> + sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
> +
> + eval "$merge_tool_path" \
> + -f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
> + fi
I wonder if there were an easy way to "compare" the $FINAL_CMD this
new code feeds to $merge_tool_path and what was fed to it by the
original code to show that we are not regressing what the end user
sees.
The "run_unit_tests()" only compares the cmd generated for each
given layout, but the original vimdiff$N didn't express them in
terms of $layout this patch introduces, so unfortunately that is not
it.
Ideas?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/3] vimdiff: new implementation with layout support
2022-03-23 16:43 ` Junio C Hamano
@ 2022-03-24 12:21 ` Fernando
2022-03-24 16:52 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Fernando @ 2022-03-24 12:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, davvid, sunshine, seth, levraiphilippeblain, rogi
> > +debug_print() {
> > + # Send message to stderr if global variable DEBUG is set to "true"
> > +
> > + if test -n "$GIT_MERGETOOL_VIMDIFF_DEBUG"
> > + then
> > + >&2 echo "$@"
> > + fi
> > +}
>
> Do we want to keep this helper, and many calls to it sprinkled in
> this file, or are they leftover cruft?
I left it in case we ever need to debug this script in the future. But if you
think it's not worth it, I can delete it. We have three options:
A) Leave it
B) Completely remove it
C) Remove the function and replace the places where it is being called by
a commented out "echo" to stderr
Let me know what you prefer.
> Style. "debug_print () {", i.e. SPACE on both sides of "()".
Sure. No problem. I'll fix all function declarations (they are all missing the
extra space before the opening parenthesis)
> > +substring() {
> > + # Return a substring of $1 containing $3 characters starting at
> > + # zero-based offset $2.
> > + #
> > + # Examples:
> > + #
> > + # substring "Hello world" 0 4 --> "Hell"
> > + # substring "Hello world" 3 4 --> "lo w"
> > + # substring "Hello world" 3 10 --> "lo world"
> > +
> > + STRING=$1
> > + START=$2
> > + LEN=$3
> > +
> > + echo "$STRING" | cut -c$(( START + 1 ))-$(( START + $LEN))
> > +}
>
> The lack of space before the second closing "))" makes it look
> inconsistent.
No problem. I'll fix it.
> We should be able to do this no external commands
> and just two variable substitutions and without relying on
> bash-isms, but the above should do.
>
In v1 of this patch series, instead of this function, I was doing this other
thing:
X=${Y:a:b}
...but that is a bash-ism, so I replaced it with what you see above ("echo" +
"cut")
I was not able to find another way of doing it using just standard POSIX shell
syntax [1] (notice that "cut" is included in IEEE Std 1003.1 [2] so it shouldn't
be an issue to rely on it)
In any case, if anyone knows how we could achieve the same without using
external commands, please let me know and I'll change it (in the meantime I'll
keep searching for alternatives too). If after a reasonable amount of time none
of us manages to find a solution I suggest to leave it as it is now.
[1] https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html
[2] https://pubs.opengroup.org/onlinepubs/009696699/utilities/cut.html
> > + if $base_present
> > + then
> > + eval "$merge_tool_path" \
> > + -f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
> > + else
> > + # If there is no BASE (example: a merge conflict in a new file
> > + # with the same name created in both braches which didn't exist
> > + # before), close all BASE windows using vim's "quit" command
> > +
> > + FINAL_CMD=$(echo "$FINAL_CMD" | \
> > + sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
> > +
> > + eval "$merge_tool_path" \
> > + -f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
> > + fi
>
>
> I wonder if there were an easy way to "compare" the $FINAL_CMD this
> new code feeds to $merge_tool_path and what was fed to it by the
> original code to show that we are not regressing what the end user
> sees.
>
> The "run_unit_tests()" only compares the cmd generated for each
> given layout, but the original vimdiff$N didn't express them in
> terms of $layout this patch introduces, so unfortunately that is not
> it.
>
> Ideas?
Before this patch series, this is what each variant fed into "$merge_tool_path":
- vimdiff:
-f -d -c '4wincmd w | wincmd J' $LOCAL $BASE $REMOTE $MERGED
- vimdiff1:
-f -d -c 'echon "..."' $LOCAL $REMOTE
- vimdiff2:
-f -d -c 'wincmd l' $LOCAL $MERGED $REMOTE
- vimdiff3:
-f -d -c 'hid | hid | hid' $LOCAL $REMOTE $BASE $MERGED
After this patch series, when one of these predefined variants is selected, a
fixed layout is chosen and translated into the final string fed into
"$merge_tool_path":
- vimdiff --> (LOCAL,BASE,REMOTE)/MERGED
-f -c "echo | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
- vimdiff1 --> @LOCAL,REMOTE
-f -c "echo | vertical split | 1b | wincmd l | 3b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
- vimdiff2 --> LOCAL,MERGED,REMOTE
-f -c "echo | vertical split | 1b | wincmd l | vertical split | 4b | wincmd l | 3b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
- vimdiff3 --> MERGED
-f -c "echo | 4b | bufdo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
Thus, we need to do two things:
1. Manually check (one time operation) that the *before* and *after* strings
are equivalent (from the point of view of vim).
2. Add a unit test that verifies that the layout associated to each variant
actually produces the string listed above. That way we make sure the
functionality does not break in the future.
Step (1) is what I have already done (but I encourage others to do the same...
the more eyes the better).
Step (2)... is already done! Notice how, on the "run_unit_tests()" function,
these layouts correspond to test cases numbers #07, #09, #01 and #10 :)
NOTE: While re-reviewing this, I noticed the layout definition for "vimdiff1"
was set to "@LOCAL,MERGED" instead of "@LOCAL,REMOTE", which is the
correct value. I'll fix this in v6.
Should this be considered enough test for backwards compatibility?
NOTE: Another option would be to *completely bypass* the layout mechanism when
using one of the old variants and, in that case, feed exactly the same
string we were feeding before... but I just remembered we decided
against that in v2... so back to the question above :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/3] vimdiff: new implementation with layout support
2022-03-24 12:21 ` Fernando
@ 2022-03-24 16:52 ` Junio C Hamano
2022-03-24 20:50 ` Fernando
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2022-03-24 16:52 UTC (permalink / raw)
To: Fernando; +Cc: git, davvid, sunshine, seth, levraiphilippeblain, rogi
Fernando <greenfoo@u92.eu> writes:
>> > +debug_print() {
>> > + # Send message to stderr if global variable DEBUG is set to "true"
>> > +
>> > + if test -n "$GIT_MERGETOOL_VIMDIFF_DEBUG"
>> > + then
>> > + >&2 echo "$@"
>> > + fi
>> > +}
>>
>> Do we want to keep this helper, and many calls to it sprinkled in
>> this file, or are they leftover cruft?
>
> I left it in case we ever need to debug this script in the future. But if you
> think it's not worth it, I can delete it. We have three options:
>
> A) Leave it
> B) Completely remove it
> C) Remove the function and replace the places where it is being called by
> a commented out "echo" to stderr
>
> Let me know what you prefer.
If you anticipate that you'd need more debugging sessions as time
goes and people want to try to come up with different layouts, (A)
is preferrable, I would think.
>> We should be able to do this no external commands
>> and just two variable substitutions and without relying on
>> bash-isms, but the above should do.
>
> In v1 of this patch series, instead of this function, I was doing this other
> thing:
>
> X=${Y:a:b}
>
> ...but that is a bash-ism, so I replaced it with what you see above ("echo" +
> "cut")
Yes, I think I was the one who spotted the bash-ism in an earlier
round, if I am not mistaken.
> I was not able to find another way of doing it using just standard POSIX shell
> syntax [1] (notice that "cut" is included in IEEE Std 1003.1 [2] so it shouldn't
> be an issue to rely on it)
Although "echo" is often built-in, "cut" and pipe mean a fork to
create an extra process, which some folks may want less of.
> In any case, if anyone knows how we could achieve the same without using
> external commands, please let me know and I'll change it (in the meantime I'll
> keep searching for alternatives too). If after a reasonable amount of time none
> of us manages to find a solution I suggest to leave it as it is now.
I already said that this is good enough for now, didn't I? But see
below, and do not use it, it should work but it is uglier than a
simple single liner pipe.
>> I wonder if there were an easy way to "compare" the $FINAL_CMD this
>> new code feeds to $merge_tool_path and what was fed to it by the
>> original code to show that we are not regressing what the end user
>> sees.
>>
>> The "run_unit_tests()" only compares the cmd generated for each
>> given layout, but the original vimdiff$N didn't express them in
>> terms of $layout this patch introduces, so unfortunately that is not
>> it.
>>
>> Ideas?
>
> Before this patch series, this is what each variant fed into "$merge_tool_path":
>
> - vimdiff:
> -f -d -c '4wincmd w | wincmd J' $LOCAL $BASE $REMOTE $MERGED
>
> - vimdiff1:
> -f -d -c 'echon "..."' $LOCAL $REMOTE
>
> - vimdiff2:
> -f -d -c 'wincmd l' $LOCAL $MERGED $REMOTE
>
> - vimdiff3:
> -f -d -c 'hid | hid | hid' $LOCAL $REMOTE $BASE $MERGED
>
> After this patch series, when one of these predefined variants is selected, a
> fixed layout is chosen and translated into the final string fed into
> "$merge_tool_path":
>
> - vimdiff --> (LOCAL,BASE,REMOTE)/MERGED
> -f -c "echo | split | vertical split | 1b | wincmd l | vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
>
> - vimdiff1 --> @LOCAL,REMOTE
> -f -c "echo | vertical split | 1b | wincmd l | 3b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
>
> - vimdiff2 --> LOCAL,MERGED,REMOTE
> -f -c "echo | vertical split | 1b | wincmd l | vertical split | 4b | wincmd l | 3b | tabdo windo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
>
> - vimdiff3 --> MERGED
> -f -c "echo | 4b | bufdo diffthis" -c "tabfirst" $LOCAL $BASE $REMOTE $MERGED
>
> Thus, we need to do two things:
>
> 1. Manually check (one time operation) that the *before* and *after* strings
> are equivalent (from the point of view of vim).
Yes, and it is a good way to convey that the author did sufficient
due diligence to note in the proposed commit log message (1) these
two sets of actual vim commands, and (2) the fact that the author
made sure these "not literally identical" command line options
produce identical results for these existing modes.
> 2. Add a unit test that verifies that the layout associated to each variant
> actually produces the string listed above. That way we make sure the
> functionality does not break in the future.
Have a comment next to each of these four layout tests what variant
from pre-layout era it correspnds to, if you haven't done so.
> Step (1) is what I have already done (but I encourage others to do the same...
> the more eyes the better).
>
> Step (2)... is already done! Notice how, on the "run_unit_tests()" function,
> these layouts correspond to test cases numbers #07, #09, #01 and #10 :)
>
> NOTE: While re-reviewing this, I noticed the layout definition for "vimdiff1"
> was set to "@LOCAL,MERGED" instead of "@LOCAL,REMOTE", which is the
> correct value. I'll fix this in v6.
>
> Should this be considered enough test for backwards compatibility?
Yes, with a bit to extend the proposed log message to help #1, and a
bit of comments next to the test cases to help #2.
Thanks.
#!/bin/sh
substring () {
STRING=$1 START=$2 LEN=$3
local strip=
while test 0 -lt "$START"
do
strip="?$strip"
START=$(($START - 1))
done
if test -n "$strip"
then
STRING=${STRING#$strip}
fi
strip=
COUNT=$(( ${#STRING} - $LEN ))
while test 0 -lt "$COUNT"
do
strip="?$strip"
COUNT=$(($COUNT - 1))
done
if test -n "$strip"
then
STRING=${STRING%$strip}
fi
echo "$STRING"
}
while read string start len
do
substring $string $start $len
done
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/3] vimdiff: new implementation with layout support
2022-03-24 16:52 ` Junio C Hamano
@ 2022-03-24 20:50 ` Fernando
0 siblings, 0 replies; 8+ messages in thread
From: Fernando @ 2022-03-24 20:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, davvid, sunshine, seth, levraiphilippeblain, rogi
> > In any case, if anyone knows how we could achieve the same without using
> > external commands, please let me know and I'll change it (in the meantime I'll
> > keep searching for alternatives too). If after a reasonable amount of time none
> > of us manages to find a solution I suggest to leave it as it is now.
>
> I already said that this is good enough for now, didn't I? But see
> below, and do not use it, it should work but it is uglier than a
> simple single liner pipe.
Hats off to the clever usage of "???..." as a regex to remove the characters
that are not part of the desired substring.
> > Should this be considered enough test for backwards compatibility?
>
> Yes, with a bit to extend the proposed log message to help #1, and a
> bit of comments next to the test cases to help #2.
Good idea. I'll add both to v6.
Thanks for your feedback!
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 2/3] vimdiff: add tool documentation
2022-03-19 9:11 [PATCH v5 0/3] vimdiff: new implementation with layout support Fernando Ramos
2022-03-19 9:11 ` [PATCH v5 1/3] " Fernando Ramos
@ 2022-03-19 9:11 ` Fernando Ramos
2022-03-19 9:11 ` [PATCH v5 3/3] vimdiff: integrate layout tests in the unit tests framework ('t' folder) Fernando Ramos
2 siblings, 0 replies; 8+ messages in thread
From: Fernando Ramos @ 2022-03-19 9:11 UTC (permalink / raw)
To: git; +Cc: gitster, davvid, sunshine, seth, levraiphilippeblain, rogi
Running 'git {merge,diff}tool --tool-help' now also prints usage
information about the vimdiff tool (and its variantes) instead of just
its name.
Two new functions ('diff_cmd_help()' and 'merge_cmd_help()') have been
added to the set of functions that each merge tool (ie. scripts found
inside "mergetools/") can overwrite to provided tool specific
information.
Right now, only 'mergetools/vimdiff' implements these functions, but
other tools are encouraged to do so in the future, specially if they
take configuration options not explained anywhere else (as it is the
case with the 'vimdiff' tool and the new 'layout' option)
Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
Documentation/git-difftool--vimdiff.txt | 40 +++++
Documentation/git-mergetool--vimdiff.txt | 195 +++++++++++++++++++++++
git-mergetool--lib.sh | 14 ++
mergetools/vimdiff | 12 ++
4 files changed, 261 insertions(+)
create mode 100644 Documentation/git-difftool--vimdiff.txt
create mode 100644 Documentation/git-mergetool--vimdiff.txt
diff --git a/Documentation/git-difftool--vimdiff.txt b/Documentation/git-difftool--vimdiff.txt
new file mode 100644
index 0000000000..5788f8e149
--- /dev/null
+++ b/Documentation/git-difftool--vimdiff.txt
@@ -0,0 +1,40 @@
+git-mergetool{litdd}vimdiff(1)
+==============================
+
+NAME
+----
+git-difftool--vimdiff - Helper script to run vim as a diff tool
+
+SYNOPSIS
+--------
+[verse]
+'git difftool' --tool=vimdiff ... (rest of 'git difftool' options)
+
+DESCRIPTION
+-----------
+When specifying '--tool=vimdiff' in 'git difftool', git will open vim with a
+with two vertical windows: LOCAL changes will be placed in the left window and
+REMOTE changes in the right one.
+
+ ------------------------------------------
+ | | |
+ | | |
+ | | |
+ | LOCAL | REMOTE |
+ | | |
+ | | |
+ | | |
+ ------------------------------------------
+
+VARIANTS
+--------
+Instead of '--tool=vimdiff', you can also use these other variants:
+
+ * '--tool=gvimdiff', to open gvim instead of vim.
+
+ * '--tool=nvimdiff', to open nvim ('neovim') instead of vim.
+
+GIT
+---
+Part of the linkgit:git[1] suite
+
diff --git a/Documentation/git-mergetool--vimdiff.txt b/Documentation/git-mergetool--vimdiff.txt
new file mode 100644
index 0000000000..fb69443d3d
--- /dev/null
+++ b/Documentation/git-mergetool--vimdiff.txt
@@ -0,0 +1,195 @@
+git-difftool{litdd}vimdiff(1)
+=============================
+
+NAME
+----
+git-mergetool--vimdiff - Helper script to run vim as a merge tool
+
+SYNOPSIS
+--------
+[verse]
+'git mergetool' --tool=vimdiff ... (rest of 'git mergetool' options)
+
+DESCRIPTION
+-----------
+When specifying '--tool=vimdiff' in 'git mergetool' git will open vim with a 4
+windows layout distributed in the following way:
+
+ ------------------------------------------
+ | | | |
+ | LOCAL | BASE | REMOTE |
+ | | | |
+ ------------------------------------------
+ | |
+ | MERGED |
+ | |
+ ------------------------------------------
+
+"LOCAL", "BASE" and "REMOTE" are read-only buffers showing the contents of the
+conflicting file in a specific git commit ("commit you are merging into",
+"common ancestor commit" and "commit you are merging from" respectively)
+
+"MERGED" is a writable buffer where you have to resolve the conflicts (using the
+other read-only buffers as a reference). Once you are done, save and exit "vim"
+as usual (":wq") or, if you want to abort, exit using ":cq".
+
+LAYOUT CONFIGURATION
+--------------------
+You can change the windows layout used by vim by setting configuration variable
+"mergetool.vimdiff.layout" which accepts a string where the following separators
+have special meaning:
+
+ - "+" is used to "open a new tab"
+ - "," is used to "open a new vertical split"
+ - "/" is used to "open a new horizontal split"
+ - "@" is used to indicate which is the file containing the final version after
+ solving the conflicts. In not present, "MERGED" will be used by default.
+
+The precedence of the operators is this one (you can use parenthesis to change
+it):
+
+ @ > + > / > ,
+
+Let's see some examples to understand how it works:
+
+ * layout = "(LOCAL,BASE,REMOTE)/MERGED"
+
+ This is exactly the same as the default layout we have already seen.
+
+ Note that "/" has precedence over "," and thus the parenthesis are not
+ needed in this case. The next layout definition is equivalent:
+
+ layout = "LOCAL,BASE,REMOTE / MERGED"
+
+ * layout = "LOCAL,MERGED,REMOTE"
+
+ If, for some reason, we are not interested in the "BASE" buffer.
+
+ ------------------------------------------
+ | | | |
+ | | | |
+ | LOCAL | MERGED | REMOTE |
+ | | | |
+ | | | |
+ ------------------------------------------
+
+ * layout = "MERGED"
+
+ Only the "MERGED" buffer will be shown. Note, however, that all the other
+ ones are still loaded in vim, and you can access them with the "buffers"
+ command.
+
+ ------------------------------------------
+ | |
+ | |
+ | MERGED |
+ | |
+ | |
+ ------------------------------------------
+
+ * layout = "@LOCAL,REMOTE"
+
+ When "MERGED" is not present in the layout, you must "mark" one of the
+ buffers with an asterisk. That will become the buffer you need to edit and
+ save after resolving the conflicts.
+
+ ------------------------------------------
+ | | |
+ | | |
+ | | |
+ | LOCAL | REMOTE |
+ | | |
+ | | |
+ | | |
+ ------------------------------------------
+
+ * layout = "LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE"
+
+ Three tabs will open: the first one is a copy of the default layout, while
+ the other two only show the differences between "BASE" and "LOCAL" and
+ "BASE" and "REMOTE" respectively.
+
+ ------------------------------------------
+ | <TAB #1> | TAB #2 | TAB #3 | |
+ ------------------------------------------
+ | | | |
+ | LOCAL | BASE | REMOTE |
+ | | | |
+ ------------------------------------------
+ | |
+ | MERGED |
+ | |
+ ------------------------------------------
+
+ ------------------------------------------
+ | TAB #1 | <TAB #2> | TAB #3 | |
+ ------------------------------------------
+ | | |
+ | | |
+ | | |
+ | BASE | LOCAL |
+ | | |
+ | | |
+ | | |
+ ------------------------------------------
+
+ ------------------------------------------
+ | TAB #1 | TAB #2 | <TAB #3> | |
+ ------------------------------------------
+ | | |
+ | | |
+ | | |
+ | BASE | REMOTE |
+ | | |
+ | | |
+ | | |
+ ------------------------------------------
+
+ * layout = "LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL/BASE/REMOTE),MERGED"
+
+ Same as the previous example, but adds a fourth tab with the same
+ information as the first tab, with a different layout.
+
+ ---------------------------------------------
+ | TAB #1 | TAB #2 | TAB #3 | <TAB #4> |
+ ---------------------------------------------
+ | LOCAL | |
+ |---------------------| |
+ | BASE | MERGED |
+ |---------------------| |
+ | REMOTE | |
+ ---------------------------------------------
+
+ Note how in the third tab definition we need to use parenthesis to make ","
+ have precedence over "/".
+
+VARIANTS
+--------
+Instead of '--tool=vimdiff', you can also use these other variants:
+
+ * '--tool=gvimdiff', to open gvim instead of vim.
+
+ * '--tool=nvimdiff', to open nvim ('neovim') instead of vim.
+
+When using these variants, in order to specify a custom layout you will have to
+set configuration variables 'mergetool.gvimdiff.layout' and
+'mergetool.nvimdiff.layout' instead of 'mergetool.vimdiff.layout'
+
+In addition, for backwards compatibility with previous git versions, you can
+also append '1', '2' or '3' to either 'vimdiff' or any of the variants (ex:
+'vimdiff3', 'nvimdiff1', etc...) to use a predefined layout.
+In other words, using '--tool=[g,n,]vimdiffx' is the same as using
+'--tool=[g,n,]vimdiff' and setting configuration variable
+'mergetool.[g,n,]vimdiff.layout' to...
+
+ * x=1 --> "@LOCAL, REMOTE"
+ * x=2 --> "LOCAL, MERGED, REMOTE"
+ * x=3 --> "MERGED"
+
+Example: using '--tool=gvimdiff2' will open 'gvim' with three columns (LOCAL,
+MERGED and REMOTE).
+
+GIT
+---
+Part of the linkgit:git[1] suite
+
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 542a6a75eb..956c276e1d 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -64,6 +64,12 @@ $(list_tool_variants)"
fi
shown_any=yes
printf "%s%s\n" "$per_line_prefix" "$toolname"
+
+ (diff_mode && diff_cmd_help "$toolname" || merge_cmd_help "$toolname") |
+ while read -r line
+ do
+ printf "%s\t%s\n" "$per_line_prefix" "$line"
+ done
fi
done
@@ -162,10 +168,18 @@ setup_tool () {
return 1
}
+ diff_cmd_help () {
+ return 0
+ }
+
merge_cmd () {
return 1
}
+ merge_cmd_help () {
+ return 0
+ }
+
hide_resolved_enabled () {
return 0
}
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 5bf77a5388..db6cb75728 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -364,6 +364,12 @@ diff_cmd () {
}
+diff_cmd_help() {
+ echo "Run 'man git-difftool--vimdiff' for details"
+ return 0
+}
+
+
merge_cmd () {
layout=$(git config mergetool.$merge_tool.layout)
@@ -435,6 +441,12 @@ merge_cmd () {
}
+merge_cmd_help() {
+ echo "Run 'man git-mergetool--vimdiff' for details"
+ return 0
+}
+
+
translate_merge_tool_path() {
case "$1" in
nvimdiff*)
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 3/3] vimdiff: integrate layout tests in the unit tests framework ('t' folder)
2022-03-19 9:11 [PATCH v5 0/3] vimdiff: new implementation with layout support Fernando Ramos
2022-03-19 9:11 ` [PATCH v5 1/3] " Fernando Ramos
2022-03-19 9:11 ` [PATCH v5 2/3] vimdiff: add tool documentation Fernando Ramos
@ 2022-03-19 9:11 ` Fernando Ramos
2 siblings, 0 replies; 8+ messages in thread
From: Fernando Ramos @ 2022-03-19 9:11 UTC (permalink / raw)
To: git; +Cc: gitster, davvid, sunshine, seth, levraiphilippeblain, rogi
Create a new test case file for the different available merge tools.
Right now it only tests the 'mergetool.vimdiff.layout' options. Other
merge tools might be interested in adding their own tests here too.
Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
t/t7609-mergetool--lib.sh | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100755 t/t7609-mergetool--lib.sh
diff --git a/t/t7609-mergetool--lib.sh b/t/t7609-mergetool--lib.sh
new file mode 100755
index 0000000000..8cfc22e27c
--- /dev/null
+++ b/t/t7609-mergetool--lib.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+test_description='git mergetool
+
+Testing basic merge tools options'
+
+. ./test-lib.sh
+
+test_expect_success 'mergetool --tool=vimdiff creates the expected layout' '
+ . $GIT_BUILD_DIR/mergetools/vimdiff &&
+ run_unit_tests
+'
+
+test_done
+
--
2.35.1
^ permalink raw reply related [flat|nested] 8+ messages in thread