* [RFC PATCH] mergetools: support difftool.tabbed setting @ 2021-01-13 5:59 Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev 2021-02-12 5:51 ` [RFC PATCH] mergetools: support difftool.tabbed setting David Aguilar 0 siblings, 2 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-13 5:59 UTC (permalink / raw) To: git [-- Attachment #1.1: Type: text/plain, Size: 1457 bytes --] I was asked how to configure "git difftool" to open files using several tabs and stop spawning diff application on every modified file. I looked into Git source and found no possibility to run diff tool at one step. The patch allows a user to view diffs in single window at one go. The current implementation is still poor and it can be used solely for demonstration purposes. To see it in action, tweak the local gitconfig: git config difftool.prompt false git config difftool.tabbed true Then run: git difftool -t vimdiff Or: git difftool -t meld The solution has some restrictions, diffing up to ten files works now (I did not bother with dynamic memory allocation), and it does not handle spaces in file names (I do not know how to pass them correctly to underlying tools without "xargs -0"). I think the git-difftool--helper should be changed so that it could process many files in single invocation and it would not use a temporary file by itself. A similar behaviour can be done in git-mergetool, too. Do you have ideas how to better implement such a feature? Any comments are welcome. P.S.: I'm attaching screenshots for a clear demo what I mean. --- diff.c | 4 ++-- git-mergetool--lib.sh | 36 +++++++++++++++++++++++++++++++++++- mergetools/meld | 4 ++++ mergetools/vimdiff | 17 +++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) [-- Attachment #1.2: 0001-mergetools-support-difftool.tabbed-setting.patch --] [-- Type: text/x-patch, Size: 3035 bytes --] diff --git a/diff.c b/diff.c index 2253ec880..8a265e0b0 100644 --- a/diff.c +++ b/diff.c @@ -532,7 +532,7 @@ static struct diff_tempfile { * this tempfile object is used to manage its lifetime. */ struct tempfile *tempfile; -} diff_temp[2]; +} diff_temp[20]; struct emit_callback { int color_diff; @@ -4275,7 +4275,7 @@ static void run_external_diff(const char *pgm, if (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v)) die(_("external diff died, stopping at %s"), name); - remove_tempfile(); + //remove_tempfile(); strvec_clear(&argv); strvec_clear(&env); } diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 7225abd81..e599e4243 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -193,6 +193,8 @@ setup_tool () { false } + unset -f diff_combo_cmd + if test -f "$MERGE_TOOLS_DIR/$tool" then . "$MERGE_TOOLS_DIR/$tool" @@ -248,6 +250,25 @@ trust_exit_code () { fi } +is_difftool_tabbed () { + : "${GIT_DIFFTOOL_TABBED:=$(git config --type=bool --default=false difftool.tabbed)}" + case $(printf "%s" "$GIT_DIFFTOOL_TABBED" | tr '[:upper:]' '[:lower:]') in + yes|on|true|1) + GIT_DIFFTOOL_TABBED=true + ;; + no|off|false|0) + GIT_DIFFTOOL_TABBED=false + ;; + *) + echo "error: bad boolean value of GIT_DIFFTOOL_TABBED" >&2 + exit 1 + ;; + esac + + test "$GIT_DIFFTOOL_TABBED" = true && test "$GIT_DIFF_PATH_TOTAL" -gt 1 \ + && type diff_combo_cmd >/dev/null 2>&1 +} + # Entry point for running tools run_merge_tool () { @@ -272,7 +293,20 @@ run_merge_tool () { # Run a either a configured or built-in diff tool run_diff_cmd () { - diff_cmd "$1" + if is_difftool_tabbed + then + temp_file="${TMPDIR:-/tmp}/git-${PPID}_tabbed-queue" + test "$GIT_DIFF_PATH_COUNTER" -eq 1 && > "$temp_file" + printf "%s " "$LOCAL" "$REMOTE" >> "$temp_file" + + if [ "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" ] + then + diff_combo_cmd 3< "$temp_file" + rm -f -- "$temp_file" + fi + else + diff_cmd "$1" + fi } # Run a either a configured or built-in merge tool diff --git a/mergetools/meld b/mergetools/meld index aab4ebb93..6570bf0f8 100644 --- a/mergetools/meld +++ b/mergetools/meld @@ -2,6 +2,10 @@ diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" } +diff_combo_cmd () { + ( IFS=' '; "$merge_tool_path" $(printf ' --diff %s %s' `cat <&3`) 3<&- ) +} + merge_cmd () { check_meld_for_features diff --git a/mergetools/vimdiff b/mergetools/vimdiff index abc8ce4ec..31d6e1eaa 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -3,6 +3,23 @@ diff_cmd () { -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" } +multitabbed_script=' + let i = 1 + while i < argc() + execute "tabedit" fnameescape(argv(i - 1)) + execute "diffsplit" fnameescape(argv(i)) + wincmd L + let i = i + 2 + endwhile + tabfirst + tabclose + unlet i +' +diff_combo_cmd () { + ( IFS=' '; cd "$GIT_PREFIX" && "$merge_tool_path" -R -f \ + -c "$multitabbed_script" `cat <&3` 3<&- ) +} + merge_cmd () { case "$1" in *vimdiff) [-- Attachment #1.3: meld_screenshot-1.png --] [-- Type: image/png, Size: 58533 bytes --] [-- Attachment #1.4: vimdiff_screenshot-2.png --] [-- Type: image/png, Size: 42323 bytes --] [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH v2 0/3] implement tabbed mode in difftools 2021-01-13 5:59 [RFC PATCH] mergetools: support difftool.tabbed setting Nicholas Guriev @ 2021-01-18 21:00 ` Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev ` (3 more replies) 2021-02-12 5:51 ` [RFC PATCH] mergetools: support difftool.tabbed setting David Aguilar 1 sibling, 4 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-18 21:00 UTC (permalink / raw) To: git In this mode the "git difftool" command opens all compared files via single invocation of an editor passing to it entire list of the changed files. I find it useful, and it allows a user to switch easily forward and back between the files. You will find some screenshots demonstrating the feature in my previous message. The patch series looks better now, and the two problems mentioned earlier have been solved. I added a new static function, forget_tempfile, into the diff.c file. It cleans diff_temp structures and do not remove the temporary files. They anyway would be deleted in an atexit handler. In this way, I can collect content of all changed files before running the editor. For solving the second problem with spaces, I am separating file names with line-feeds, '\n', that are less common. This restriction is the same with the "git mergetool" command. I think it is acceptable at the present stage. I have also repaired prompting in the tabbed mode. In such case, the command asks a user only once right before starting the editor. And I have described briefly the new changes to the best of my ability. Alas, some other problems still remain. First, some automated tests failed that related to the difftool.prompt setting. I have modified its behavior a bit and it now contains a loop. Second, there are no new tests, yet I am going to add them later. And third, the main remaining problem is the method of calling of the difftool helper. I do not like that I have to write temporary files in the helper code to store its state between invocations. But maybe, someone offer a better solution that is easy to incorporate into the current architecture. In conclusion, the patches are still not ready for merging, and any comments or testing are welcome. Nicholas Guriev (3): mergetools: support difftool.tabbed setting difftool-helper: conciliate difftool.tabbed and difftool.prompt settings doc: describe new difftool.tabbed feature Documentation/config/difftool.txt | 6 +++ Documentation/git-difftool.txt | 19 ++++++++-- Documentation/git.txt | 4 ++ builtin/difftool.c | 7 +++- diff.c | 10 ++++- git-difftool--helper.sh | 39 ++++++++++++------- git-mergetool--lib.sh | 62 ++++++++++++++++++++++++++++++- mergetools/meld | 4 ++ mergetools/vimdiff | 17 +++++++++ 9 files changed, 148 insertions(+), 20 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev @ 2021-01-18 21:00 ` Nicholas Guriev 2021-01-18 23:25 ` Junio C Hamano 2021-01-18 21:00 ` [RFC PATCH v2 2/3] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Nicholas Guriev @ 2021-01-18 21:00 UTC (permalink / raw) To: git The patch introduces a new boolean setting, difftool.tabbed, which allows a user to view diffs in single window at one go. The `--tabbed` command line option and the GIT_DIFFTOOL_TABBED environment variable are also available. For now, it works only with vimdiff and related, meld. We call to a new invented function, diff_combo_cmd, if it is provided by the diff tool, and pass to it a list of pairs of compared files. The list is available through third file descriptor, filenames are separated by a line feed, '\n', so they can be split by means of unmodified $IFS. The function may close that descriptor after reading the list. --- builtin/difftool.c | 7 ++++++- diff.c | 10 +++++++++- git-mergetool--lib.sh | 42 +++++++++++++++++++++++++++++++++++++++++- mergetools/meld | 4 ++++ mergetools/vimdiff | 17 +++++++++++++++++ 5 files changed, 77 insertions(+), 3 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index 6e18e623fd..f061d3c029 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -689,7 +689,7 @@ static int run_file_diff(int prompt, const char *prefix, int cmd_difftool(int argc, const char **argv, const char *prefix) { int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0, - tool_help = 0, no_index = 0; + tabbed = -1, tool_help = 0, no_index = 0; static char *difftool_cmd = NULL, *extcmd = NULL; struct option builtin_difftool_options[] = { OPT_BOOL('g', "gui", &use_gui_tool, @@ -708,6 +708,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "tool-help", &tool_help, N_("print a list of diff tools that may be used with " "`--tool`")), + OPT_BOOL_F(0, "tabbed", &tabbed, + N_("use tabs in diff tool if supported"), 0), OPT_BOOL(0, "trust-exit-code", &trust_exit_code, N_("make 'git-difftool' exit when an invoked diff " "tool returns a non - zero exit code")), @@ -756,6 +758,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) die(_("no <cmd> given for --extcmd=<cmd>")); } + if (tabbed >= 0) + setenv("GIT_DIFFTOOL_TABBED", tabbed ? "true" : "false", 1); + setenv("GIT_DIFFTOOL_TRUST_EXIT_CODE", trust_exit_code ? "true" : "false", 1); diff --git a/diff.c b/diff.c index 2253ec8802..2868848bba 100644 --- a/diff.c +++ b/diff.c @@ -1730,6 +1730,14 @@ static struct diff_tempfile *claim_diff_tempfile(void) BUG("diff is failing to clean up its tempfiles"); } +static void forget_tempfile(void) +{ + for (int i = 0; i < ARRAY_SIZE(diff_temp); i++) { + close_tempfile_gently(diff_temp[i].tempfile); + diff_temp[i] = (struct diff_tempfile){0}; + } +} + static void remove_tempfile(void) { int i; @@ -4275,7 +4283,7 @@ static void run_external_diff(const char *pgm, if (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v)) die(_("external diff died, stopping at %s"), name); - remove_tempfile(); + forget_tempfile(); strvec_clear(&argv); strvec_clear(&env); } diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 78f3647ed9..2a1edbb9b9 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -195,6 +195,11 @@ setup_tool () { false } + # Clear combo function declared by a previous tool (if any) to + # unambiguously indicate that the current one supports the feature or + # not. + unset -f diff_combo_cmd + if test -f "$MERGE_TOOLS_DIR/$tool" then . "$MERGE_TOOLS_DIR/$tool" @@ -250,6 +255,28 @@ trust_exit_code () { fi } +# Check whether tabbed mode is requested and current loaded tool supports it. +is_difftool_tabbed () { + : "${GIT_DIFFTOOL_TABBED=$(git config --type=bool \ + --default=false difftool.tabbed || echo bad value)}" + case $(printf "%s" "$GIT_DIFFTOOL_TABBED" | tr '[:upper:]' '[:lower:]') in + yes|on|true|1) + GIT_DIFFTOOL_TABBED=true + ;; + no|off|false|0|'') + GIT_DIFFTOOL_TABBED=false + ;; + *) + echo "error: bad boolean value of GIT_DIFFTOOL_TABBED" >&2 + exit 1 + ;; + esac + + test "$GIT_DIFFTOOL_TABBED" = true && + test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 && + type diff_combo_cmd >/dev/null 2>&1 +} + # Entry point for running tools run_merge_tool () { @@ -274,7 +301,20 @@ run_merge_tool () { # Run a either a configured or built-in diff tool run_diff_cmd () { - diff_cmd "$1" + if is_difftool_tabbed + then + temp_file="${TMPDIR:-/tmp}/git-${PPID}_tabbed-queue" + test "$GIT_DIFF_PATH_COUNTER" -eq 1 && >"$temp_file" + printf '%s\n' "$LOCAL" "$REMOTE" >>"$temp_file" + + if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" + then + diff_combo_cmd 3<"$temp_file" + rm -f -- "$temp_file" + fi + else + diff_cmd "$1" + fi } # Run a either a configured or built-in merge tool diff --git a/mergetools/meld b/mergetools/meld index aab4ebb935..2f40262a70 100644 --- a/mergetools/meld +++ b/mergetools/meld @@ -2,6 +2,10 @@ diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" } +diff_combo_cmd () { + "$merge_tool_path" $(printf -- '--diff\n%s\n%s\n' `cat <&3`) 3<&- +} + merge_cmd () { check_meld_for_features diff --git a/mergetools/vimdiff b/mergetools/vimdiff index abc8ce4ec4..006e7b825d 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -3,6 +3,23 @@ diff_cmd () { -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" } +multitabbed_script=' + let i = 1 + while i < argc() + execute "tabedit" fnameescape(argv(i - 1)) + execute "diffsplit" fnameescape(argv(i)) + wincmd L + let i = i + 2 + endwhile + tabfirst + tabclose + unlet i + chdir $GIT_PREFIX +' +diff_combo_cmd () { + "$merge_tool_path" -R -f -c "$multitabbed_script" -- `cat <&3` 3<&- +} + merge_cmd () { case "$1" in *vimdiff) -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting 2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev @ 2021-01-18 23:25 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-01-18 23:25 UTC (permalink / raw) To: Nicholas Guriev; +Cc: git Nicholas Guriev <nicholas@guriev.su> writes: > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 78f3647ed9..2a1edbb9b9 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -195,6 +195,11 @@ setup_tool () { > false > } > > + # Clear combo function declared by a previous tool (if any) to > + # unambiguously indicate that the current one supports the feature or > + # not. > + unset -f diff_combo_cmd > + I know this is so that you can use "type diff_combo_cmd" in a different part of the patch, but I do not think you want to be fooled by an unrelated ~/bin/diff_combo_cmd that the user may have under the home directory. Instead, the fallback definitions can have diff_combo_supported () { return 1 } and have backends that does support diff_combo_supported to override with their own diff_combo_supported () { return 0 } And then, the part of is_difftool_tabbed that wants to see if the current backend supports the operation can become: test "$GIT_DIFFTOOL_TABBED" = true && test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 && - type diff_combo_cmd >/dev/null 2>&1 + diff_combo_supported } That way, you do not have to do "unset -f" up above. > @@ -250,6 +255,28 @@ trust_exit_code () { > fi > } > > +# Check whether tabbed mode is requested and current loaded tool supports it. > +is_difftool_tabbed () { > + : "${GIT_DIFFTOOL_TABBED=$(git config --type=bool \ > + --default=false difftool.tabbed || echo bad value)}" > + case $(printf "%s" "$GIT_DIFFTOOL_TABBED" | tr '[:upper:]' '[:lower:]') in > + yes|on|true|1) > + GIT_DIFFTOOL_TABBED=true > + ;; > + no|off|false|0|'') > + GIT_DIFFTOOL_TABBED=false > + ;; > + *) > + echo "error: bad boolean value of GIT_DIFFTOOL_TABBED" >&2 > + exit 1 > + ;; > + esac > + > + test "$GIT_DIFFTOOL_TABBED" = true && > + test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 && > + type diff_combo_cmd >/dev/null 2>&1 > +} ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH v2 2/3] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev @ 2021-01-18 21:00 ` Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 3/3] doc: describe new difftool.tabbed feature Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev 3 siblings, 0 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-18 21:00 UTC (permalink / raw) To: git The commit combines paths of compared files into a separate temporary file and prints them before launch the tool on last invocation of the helper. All temporary files will be removed before exiting after that. At least, we try. But it may happen the files remain in case of a bug or a random SIGKILL. --- git-difftool--helper.sh | 39 ++++++++++++++++++++++++++------------- git-mergetool--lib.sh | 34 +++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 46af3e60b7..529d55c96d 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -11,8 +11,8 @@ TOOL_MODE=diff # difftool.prompt controls the default prompt/no-prompt behavior # and is overridden with $GIT_DIFFTOOL*_PROMPT. should_prompt () { - prompt_merge=$(git config --bool mergetool.prompt || echo true) - prompt=$(git config --bool difftool.prompt || echo $prompt_merge) + prompt=$(git config --bool mergetool.prompt || echo true) + prompt=$(git config --bool difftool.prompt || echo $prompt) if test "$prompt" = true then test -z "$GIT_DIFFTOOL_NO_PROMPT" @@ -26,6 +26,18 @@ use_ext_cmd () { test -n "$GIT_DIFFTOOL_EXTCMD" } +prompt_before_launch () { + while true + do + printf "Launch '%s' [Y/n]? " "${GIT_DIFFTOOL_EXTCMD:-$merge_tool}" + read ans 2>/dev/null || return 1 + case "${ans-y}" in + [yY]*) return 0 ;; + [nN]*) return 1 ;; + esac + done +} + launch_merge_tool () { # Merged is the filename as it appears in the work tree # Local is the contents of a/filename @@ -40,19 +52,20 @@ launch_merge_tool () { # the user with the real $MERGED name before launching $merge_tool. if should_prompt then - printf "\nViewing (%s/%s): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \ - "$GIT_DIFF_PATH_TOTAL" "$MERGED" - if use_ext_cmd + append_templist merged-paths "$MERGED" + # Do preinit before test whether the tool supports tabbed run. + use_ext_cmd || setup_tool "$merge_tool" + + if ! is_difftool_tabbed then - printf "Launch '%s' [Y/n]? " \ - "$GIT_DIFFTOOL_EXTCMD" - else - printf "Launch '%s' [Y/n]? " "$merge_tool" - fi - read ans || return - if test "$ans" = n + printf "\nViewing (%d/%d): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \ + "$GIT_DIFF_PATH_TOTAL" "$MERGED" + prompt_before_launch || return + elif on_last_file then - return + printf "Viewing %d files:\n" "$GIT_DIFF_PATH_TOTAL" + printf " '%s'\n" `cat "$LAST_TEMPFILE"` + prompt_before_launch || return fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 2a1edbb9b9..4bb0e31dac 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -22,6 +22,10 @@ is_available () { type "$merge_tool_path" >/dev/null 2>&1 } +on_last_file () { + test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" 2>/dev/null +} + list_config_tools () { section=$1 line_prefix=${2:-} @@ -277,6 +281,26 @@ is_difftool_tabbed () { type diff_combo_cmd >/dev/null 2>&1 } +# Save lines to the chosen temporary file for usage from subsequent invocations. +# Path to the file will be placed to the LAST_TEMPFILE variable for later links. +append_templist () { + LAST_TEMPFILE="${TMPDIR:-/tmp}/git-${PPID}_$1" + shift + + if test "$GIT_DIFF_PATH_COUNTER" -eq 1 + then + printf '%s\n' "$@" >"$LAST_TEMPFILE" + else + printf '%s\n' "$@" >>"$LAST_TEMPFILE" + fi +} + +# Clean any temporary files that may create this script. +clean_templists () { + rm -f -- "${TMPDIR:-/tmp}/git-${PPID}"_* +} +trap 'on_last_file && clean_templists' EXIT INT TERM + # Entry point for running tools run_merge_tool () { @@ -303,14 +327,10 @@ run_merge_tool () { run_diff_cmd () { if is_difftool_tabbed then - temp_file="${TMPDIR:-/tmp}/git-${PPID}_tabbed-queue" - test "$GIT_DIFF_PATH_COUNTER" -eq 1 && >"$temp_file" - printf '%s\n' "$LOCAL" "$REMOTE" >>"$temp_file" - - if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" + append_templist tabbed-queue "$LOCAL" "$REMOTE" + if on_last_file then - diff_combo_cmd 3<"$temp_file" - rm -f -- "$temp_file" + diff_combo_cmd 3<"$LAST_TEMPFILE" fi else diff_cmd "$1" -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH v2 3/3] doc: describe new difftool.tabbed feature 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 2/3] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev @ 2021-01-18 21:00 ` Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev 3 siblings, 0 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-18 21:00 UTC (permalink / raw) To: git And related the `--tabbed` command line option and the GIT_DIFFTOOL_TABBED environment variable. --- Documentation/config/difftool.txt | 6 ++++++ Documentation/git-difftool.txt | 19 +++++++++++++++---- Documentation/git.txt | 4 ++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/config/difftool.txt b/Documentation/config/difftool.txt index 6762594480..ac609aee66 100644 --- a/Documentation/config/difftool.txt +++ b/Documentation/config/difftool.txt @@ -12,3 +12,9 @@ difftool.<tool>.cmd:: difftool.prompt:: Prompt before each invocation of the diff tool. + +difftool.tabbed:: + Show compared files in different tabs using single invocation of + the diff tool. Must be a boolean value. Only the following tools + are currently supported: vimdiff and related, meld. Tools with + overridden command line will ignore this configuration variable. diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 484c485fd0..1b7a5345ad 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -69,6 +69,13 @@ with custom merge tool commands and has the same value as `$MERGED`. --tool-help:: Print a list of diff tools that may be used with `--tool`. +--[no-]tabbed:: + Open compared files in different tabs using single invocation + of the diff tool. This overrides configuration or environment. + Currently, only the following tools are supported: vimdiff and + related, meld. Tools with overridden command line will ignore + this option. + --[no-]symlinks:: 'git difftool''s default behavior is create symlinks to the working tree when run in `--dir-diff` mode and the right-hand @@ -95,10 +102,11 @@ instead. `--no-symlinks` is the default on Windows. `diff.tool`, `merge.tool` until a tool is found. --[no-]trust-exit-code:: - 'git-difftool' invokes a diff tool individually on each file. - Errors reported by the diff tool are ignored by default. - Use `--trust-exit-code` to make 'git-difftool' exit when an - invoked diff tool returns a non-zero exit code. + 'git-difftool' invokes a diff tool individually on each file + unless tabbed mode is active. Errors reported by the diff tool + are ignored by default. Use `--trust-exit-code` to make + 'git-difftool' exit immediately when an invoked diff tool + returns a non-zero exit code. + 'git-difftool' will forward the exit code of the invoked tool when `--trust-exit-code` is used. @@ -128,6 +136,9 @@ See the `--tool=<tool>` option above for more details. difftool.prompt:: Prompt before each invocation of the diff tool. +difftool.tabbed:: + Configure default value of the `--tabbed` option. See above. + difftool.trustExitCode:: Exit difftool if the invoked diff tool returns a non-zero exit status. + diff --git a/Documentation/git.txt b/Documentation/git.txt index a6d4ad0818..d7cd6650c9 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -583,6 +583,10 @@ For each path `GIT_EXTERNAL_DIFF` is called, two environment variables, `GIT_DIFF_PATH_TOTAL`:: The total number of paths. +`GIT_DIFFTOOL_TABBED`:: + Run the diff tool in tabbed mode opening all compared files + together. It must contain a boolean value. + other ~~~~~ `GIT_MERGE_VERBOSITY`:: -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 0/4] difftools in tabbed mode 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev ` (2 preceding siblings ...) 2021-01-18 21:00 ` [RFC PATCH v2 3/3] doc: describe new difftool.tabbed feature Nicholas Guriev @ 2021-01-25 21:21 ` Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 1/4] mergetools: support difftool.tabbed setting Nicholas Guriev ` (3 more replies) 3 siblings, 4 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-25 21:21 UTC (permalink / raw) To: git Junio C Hamano, thank you for the comment. I added the diff_combo_supported function as you suggested. Though it calls to the `true` or the `false` builtins as I find them much clearer than the "return" command with inverted status. It is really confusing that C and most other programming languages adequately interpret non-zero as truth value and zero as falsity value unlike Unix Shell. Are such changes acceptable? As regards broken tests, it may seem silly but that error was due to a single missing colon. I corrected the mistake and added some automated tests of this new feature. diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 529d55c96d..85a6dc9c30 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -31,7 +31,7 @@ prompt_before_launch () { do printf "Launch '%s' [Y/n]? " "${GIT_DIFFTOOL_EXTCMD:-$merge_tool}" read ans 2>/dev/null || return 1 - case "${ans-y}" in + case "${ans:-y}" in [yY]*) return 0 ;; [nN]*) return 1 ;; esac Changes compared to v2: * Introduced the diff_combo_supported function. * Fixed default answer to difftool prompting. * More docs about the diff tool mechanism. * New auto-tests of the feature. Nicholas Guriev (4): mergetools: support difftool.tabbed setting difftool-helper: conciliate difftool.tabbed and difftool.prompt settings doc: describe new difftool.tabbed feature t7800: new tests of difftool.tabbed feature Documentation/config/difftool.txt | 6 + Documentation/git-difftool.txt | 19 ++- Documentation/git-mergetool--lib.txt | 62 ++++++++++ Documentation/git.txt | 8 ++ builtin/difftool.c | 7 +- diff.c | 10 +- git-difftool--helper.sh | 39 ++++-- git-mergetool--lib.sh | 71 ++++++++++- mergetools/meld | 8 ++ mergetools/vimdiff | 21 ++++ t/t7800-difftool.sh | 175 ++++++++++++++++++++++++++- 11 files changed, 401 insertions(+), 25 deletions(-) -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 1/4] mergetools: support difftool.tabbed setting 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev @ 2021-01-25 21:21 ` Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-25 21:21 UTC (permalink / raw) To: git The patch introduces a new boolean setting, difftool.tabbed, which allows a user to view diffs in single window at one go. The `--tabbed` command line option and the GIT_DIFFTOOL_TABBED environment variable are also available. For now, it works only with vimdiff and related, meld. We call to a new invented function, diff_combo_cmd, if it is provided by the diff tool, and pass to it a list of pairs of compared files. The list is available through third file descriptor, filenames are separated by a line feed, '\n', so they can be split by means of unmodified $IFS. The function may close that descriptor after reading the list. Signed-off-by: Nicholas Guriev <nicholas@guriev.su> --- builtin/difftool.c | 7 +++++- diff.c | 10 ++++++++- git-mergetool--lib.sh | 51 ++++++++++++++++++++++++++++++++++++++----- mergetools/meld | 8 +++++++ mergetools/vimdiff | 21 ++++++++++++++++++ 5 files changed, 90 insertions(+), 7 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index 6e18e623fd..f061d3c029 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -689,7 +689,7 @@ static int run_file_diff(int prompt, const char *prefix, int cmd_difftool(int argc, const char **argv, const char *prefix) { int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0, - tool_help = 0, no_index = 0; + tabbed = -1, tool_help = 0, no_index = 0; static char *difftool_cmd = NULL, *extcmd = NULL; struct option builtin_difftool_options[] = { OPT_BOOL('g', "gui", &use_gui_tool, @@ -708,6 +708,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "tool-help", &tool_help, N_("print a list of diff tools that may be used with " "`--tool`")), + OPT_BOOL_F(0, "tabbed", &tabbed, + N_("use tabs in diff tool if supported"), 0), OPT_BOOL(0, "trust-exit-code", &trust_exit_code, N_("make 'git-difftool' exit when an invoked diff " "tool returns a non - zero exit code")), @@ -756,6 +758,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) die(_("no <cmd> given for --extcmd=<cmd>")); } + if (tabbed >= 0) + setenv("GIT_DIFFTOOL_TABBED", tabbed ? "true" : "false", 1); + setenv("GIT_DIFFTOOL_TRUST_EXIT_CODE", trust_exit_code ? "true" : "false", 1); diff --git a/diff.c b/diff.c index 2253ec8802..2868848bba 100644 --- a/diff.c +++ b/diff.c @@ -1730,6 +1730,14 @@ static struct diff_tempfile *claim_diff_tempfile(void) BUG("diff is failing to clean up its tempfiles"); } +static void forget_tempfile(void) +{ + for (int i = 0; i < ARRAY_SIZE(diff_temp); i++) { + close_tempfile_gently(diff_temp[i].tempfile); + diff_temp[i] = (struct diff_tempfile){0}; + } +} + static void remove_tempfile(void) { int i; @@ -4275,7 +4283,7 @@ static void run_external_diff(const char *pgm, if (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v)) die(_("external diff died, stopping at %s"), name); - remove_tempfile(); + forget_tempfile(); strvec_clear(&argv); strvec_clear(&env); } diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 78f3647ed9..fef9289618 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -151,19 +151,25 @@ setup_tool () { # Fallback definitions, to be overridden by tools. can_merge () { - return 0 + true } can_diff () { - return 0 + true } diff_cmd () { - return 1 + false + } + + # The concrete diff tool may define that it supports combined + # invocations and the diff_combo_cmd function may be called. + diff_combo_supported () { + false } merge_cmd () { - return 1 + false } translate_merge_tool_path () { @@ -250,6 +256,28 @@ trust_exit_code () { fi } +# Check whether tabbed mode is requested and current loaded tool supports it. +is_difftool_tabbed () { + : "${GIT_DIFFTOOL_TABBED=$(git config --type=bool \ + --default=false difftool.tabbed || echo bad value)}" + case $(printf "%s" "$GIT_DIFFTOOL_TABBED" | tr '[:upper:]' '[:lower:]') in + yes|on|true|1) + GIT_DIFFTOOL_TABBED=true + ;; + no|off|false|0|'') + GIT_DIFFTOOL_TABBED=false + ;; + *) + echo "error: bad boolean value of GIT_DIFFTOOL_TABBED" >&2 + exit 1 + ;; + esac + + test "$GIT_DIFFTOOL_TABBED" = true && + test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 && + diff_combo_supported +} + # Entry point for running tools run_merge_tool () { @@ -274,7 +302,20 @@ run_merge_tool () { # Run a either a configured or built-in diff tool run_diff_cmd () { - diff_cmd "$1" + if is_difftool_tabbed + then + temp_file="${TMPDIR:-/tmp}/git-${PPID}_tabbed-queue" + test "$GIT_DIFF_PATH_COUNTER" -eq 1 && >"$temp_file" + printf '%s\n' "$LOCAL" "$REMOTE" >>"$temp_file" + + if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" + then + diff_combo_cmd 3<"$temp_file" + rm -f -- "$temp_file" + fi + else + diff_cmd "$1" + fi } # Run a either a configured or built-in merge tool diff --git a/mergetools/meld b/mergetools/meld index aab4ebb935..e2b9d456c1 100644 --- a/mergetools/meld +++ b/mergetools/meld @@ -2,6 +2,14 @@ diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" } +diff_combo_cmd () { + "$merge_tool_path" $(printf -- '--diff\n%s\n%s\n' `cat <&3`) 3<&- +} + +diff_combo_supported () { + true +} + merge_cmd () { check_meld_for_features diff --git a/mergetools/vimdiff b/mergetools/vimdiff index abc8ce4ec4..fe1923076b 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -3,6 +3,27 @@ diff_cmd () { -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" } +multitabbed_script=' + let i = 1 + while i < argc() + execute "tabedit" fnameescape(argv(i - 1)) + execute "diffsplit" fnameescape(argv(i)) + wincmd L + let i = i + 2 + endwhile + tabfirst + tabclose + unlet i + chdir $GIT_PREFIX +' +diff_combo_cmd () { + "$merge_tool_path" -R -f -c "$multitabbed_script" -- `cat <&3` 3<&- +} + +diff_combo_supported () { + true +} + merge_cmd () { case "$1" in *vimdiff) -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 1/4] mergetools: support difftool.tabbed setting Nicholas Guriev @ 2021-01-25 21:21 ` Nicholas Guriev 2021-01-25 23:04 ` Junio C Hamano 2021-01-25 21:21 ` [PATCH v3 3/4] doc: describe new difftool.tabbed feature Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 4/4] t7800: new tests of " Nicholas Guriev 3 siblings, 1 reply; 14+ messages in thread From: Nicholas Guriev @ 2021-01-25 21:21 UTC (permalink / raw) To: git The commit combines paths of compared files into a separate temporary file and prints them before launch the tool on last invocation of the helper. All temporary files will be removed before exiting after that. At least, we try. But it may happen the files remain in case of a bug or a random SIGKILL. Signed-off-by: Nicholas Guriev <nicholas@guriev.su> --- git-difftool--helper.sh | 39 ++++++++++++++++++++++++++------------- git-mergetool--lib.sh | 34 +++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 46af3e60b7..85a6dc9c30 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -11,8 +11,8 @@ TOOL_MODE=diff # difftool.prompt controls the default prompt/no-prompt behavior # and is overridden with $GIT_DIFFTOOL*_PROMPT. should_prompt () { - prompt_merge=$(git config --bool mergetool.prompt || echo true) - prompt=$(git config --bool difftool.prompt || echo $prompt_merge) + prompt=$(git config --bool mergetool.prompt || echo true) + prompt=$(git config --bool difftool.prompt || echo $prompt) if test "$prompt" = true then test -z "$GIT_DIFFTOOL_NO_PROMPT" @@ -26,6 +26,18 @@ use_ext_cmd () { test -n "$GIT_DIFFTOOL_EXTCMD" } +prompt_before_launch () { + while true + do + printf "Launch '%s' [Y/n]? " "${GIT_DIFFTOOL_EXTCMD:-$merge_tool}" + read ans 2>/dev/null || return 1 + case "${ans:-y}" in + [yY]*) return 0 ;; + [nN]*) return 1 ;; + esac + done +} + launch_merge_tool () { # Merged is the filename as it appears in the work tree # Local is the contents of a/filename @@ -40,19 +52,20 @@ launch_merge_tool () { # the user with the real $MERGED name before launching $merge_tool. if should_prompt then - printf "\nViewing (%s/%s): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \ - "$GIT_DIFF_PATH_TOTAL" "$MERGED" - if use_ext_cmd + append_templist merged-paths "$MERGED" + # Do preinit before test whether the tool supports tabbed run. + use_ext_cmd || setup_tool "$merge_tool" + + if ! is_difftool_tabbed then - printf "Launch '%s' [Y/n]? " \ - "$GIT_DIFFTOOL_EXTCMD" - else - printf "Launch '%s' [Y/n]? " "$merge_tool" - fi - read ans || return - if test "$ans" = n + printf "\nViewing (%d/%d): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \ + "$GIT_DIFF_PATH_TOTAL" "$MERGED" + prompt_before_launch || return + elif on_last_file then - return + printf "Viewing %d files:\n" "$GIT_DIFF_PATH_TOTAL" + printf " '%s'\n" `cat "$LAST_TEMPFILE"` + prompt_before_launch || return fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index fef9289618..c29abfa4fb 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -22,6 +22,10 @@ is_available () { type "$merge_tool_path" >/dev/null 2>&1 } +on_last_file () { + test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" 2>/dev/null +} + list_config_tools () { section=$1 line_prefix=${2:-} @@ -278,6 +282,26 @@ is_difftool_tabbed () { diff_combo_supported } +# Save lines to the chosen temporary file for usage from subsequent invocations. +# Path to the file will be placed to the LAST_TEMPFILE variable for later links. +append_templist () { + LAST_TEMPFILE="${TMPDIR:-/tmp}/git-${PPID}_$1" + shift + + if test "$GIT_DIFF_PATH_COUNTER" -eq 1 + then + printf '%s\n' "$@" >"$LAST_TEMPFILE" + else + printf '%s\n' "$@" >>"$LAST_TEMPFILE" + fi +} + +# Clean any temporary files that may create this script. +clean_templists () { + rm -f -- "${TMPDIR:-/tmp}/git-${PPID}"_* +} +trap 'on_last_file && clean_templists' EXIT INT TERM + # Entry point for running tools run_merge_tool () { @@ -304,14 +328,10 @@ run_merge_tool () { run_diff_cmd () { if is_difftool_tabbed then - temp_file="${TMPDIR:-/tmp}/git-${PPID}_tabbed-queue" - test "$GIT_DIFF_PATH_COUNTER" -eq 1 && >"$temp_file" - printf '%s\n' "$LOCAL" "$REMOTE" >>"$temp_file" - - if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL" + append_templist tabbed-queue "$LOCAL" "$REMOTE" + if on_last_file then - diff_combo_cmd 3<"$temp_file" - rm -f -- "$temp_file" + diff_combo_cmd 3<"$LAST_TEMPFILE" fi else diff_cmd "$1" -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings 2021-01-25 21:21 ` [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev @ 2021-01-25 23:04 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-01-25 23:04 UTC (permalink / raw) To: Nicholas Guriev; +Cc: git Nicholas Guriev <nicholas@guriev.su> writes: > The commit combines paths of compared files into a separate temporary > file and prints them before launch the tool on last invocation of the > helper. All temporary files will be removed before exiting after that. > At least, we try. But it may happen the files remain in case of a bug > or a random SIGKILL. > > Signed-off-by: Nicholas Guriev <nicholas@guriev.su> > --- Isn't the introduction of prompt_before_launch etc. would be a welcome change before step [v3 1/4]? If it makes a pure improvement of the existing code without adding any new/additional feature like the "tabbed" stuff, it would be better be discussed before anything else in the series. Then the new "tabbed" stuff can take advantage of the cleaned up code with the new helpers---the patch that introduces the feature will become more concise and easier to follow, I would think. If I am reading the patches right, [v3 1&2/4] goes the other way around: "let's add a new feature first in an ad-hoc way. ah, it seems that the code added for the new feature can be simplified if we tweak a few things, and while we are doing so, we can adjust old feature to use the same simplification again". Regarding what [v3 1/4] does (which if we follow thru the above suggestion would become [v4 2/4]), I am not sure that we want to add 'if tabbed mode, do this, otherwise do that' to run_diff_cmd. It would become awkward to keep piling such a change on top in the function in the longer run. I am wondering if each mergetool can override run_diff_cmd as a whole, when it is dot-sourced to set up itself. The logic you added inside is_difftool_tabbed in [v3 1/4] would probably need to be made available to the dot-sourced backends so that they can just call something without reimplementing the same logic, but by doing so, diff_combo_supported would become unneeded. Tools that do not support the "combo" mode just do not have to do anything. But such a change would become easier to review if we do restructure like this [v3 2/4] does earlier in the series _before_ a new feature gets introduced. Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/4] doc: describe new difftool.tabbed feature 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 1/4] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev @ 2021-01-25 21:21 ` Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 4/4] t7800: new tests of " Nicholas Guriev 3 siblings, 0 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-25 21:21 UTC (permalink / raw) To: git And related the `--tabbed` command line option, and the GIT_DIFFTOOL_TABBED environment variable. Signed-off-by: Nicholas Guriev <nicholas@guriev.su> --- Documentation/config/difftool.txt | 6 +++ Documentation/git-difftool.txt | 19 +++++++-- Documentation/git-mergetool--lib.txt | 62 ++++++++++++++++++++++++++++ Documentation/git.txt | 8 ++++ 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/Documentation/config/difftool.txt b/Documentation/config/difftool.txt index 6762594480..ac609aee66 100644 --- a/Documentation/config/difftool.txt +++ b/Documentation/config/difftool.txt @@ -12,3 +12,9 @@ difftool.<tool>.cmd:: difftool.prompt:: Prompt before each invocation of the diff tool. + +difftool.tabbed:: + Show compared files in different tabs using single invocation of + the diff tool. Must be a boolean value. Only the following tools + are currently supported: vimdiff and related, meld. Tools with + overridden command line will ignore this configuration variable. diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 484c485fd0..1b7a5345ad 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -69,6 +69,13 @@ with custom merge tool commands and has the same value as `$MERGED`. --tool-help:: Print a list of diff tools that may be used with `--tool`. +--[no-]tabbed:: + Open compared files in different tabs using single invocation + of the diff tool. This overrides configuration or environment. + Currently, only the following tools are supported: vimdiff and + related, meld. Tools with overridden command line will ignore + this option. + --[no-]symlinks:: 'git difftool''s default behavior is create symlinks to the working tree when run in `--dir-diff` mode and the right-hand @@ -95,10 +102,11 @@ instead. `--no-symlinks` is the default on Windows. `diff.tool`, `merge.tool` until a tool is found. --[no-]trust-exit-code:: - 'git-difftool' invokes a diff tool individually on each file. - Errors reported by the diff tool are ignored by default. - Use `--trust-exit-code` to make 'git-difftool' exit when an - invoked diff tool returns a non-zero exit code. + 'git-difftool' invokes a diff tool individually on each file + unless tabbed mode is active. Errors reported by the diff tool + are ignored by default. Use `--trust-exit-code` to make + 'git-difftool' exit immediately when an invoked diff tool + returns a non-zero exit code. + 'git-difftool' will forward the exit code of the invoked tool when `--trust-exit-code` is used. @@ -128,6 +136,9 @@ See the `--tool=<tool>` option above for more details. difftool.prompt:: Prompt before each invocation of the diff tool. +difftool.tabbed:: + Configure default value of the `--tabbed` option. See above. + difftool.trustExitCode:: Exit difftool if the invoked diff tool returns a non-zero exit status. + diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 4da9d24096..1b9fb3591e 100644 --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ -44,6 +44,68 @@ run_merge_tool:: '$MERGED', '$LOCAL', '$REMOTE', and '$BASE' must be defined for use by the merge tool. +TOOLS +----- + +There are several built-in merge tool wrappers which are located in the +'$(git --exec-path)/mergetools' directory. They are shell scripts and provide +a unified interface for the discussed scriptlet. It expects to find the +following functions defined by the each wrapper. However, most of them have +sane default implementation and the wrapper may write less boilerplate. + +can_merge:: + returns zero status (true) if the tool can be used by `git mergetool`, + otherwise the command will be unavailable. Default: true. + +can_diff:: + returns zero status (true) if the tool can be used by `git difftool`, + otherwise the command will be unavailable. Default: true. + +merge_cmd:: + should actually launch the tool in merging mode for a single path. + Positional argument: '$1' -- name of the merge tool. Predefined + variables: '$MERGED', '$LOCAL', '$REMOTE', and '$BASE' mentioned above, + '$merge_tool_path' -- absolute path to the binary of the tool or its name + if seen in default search path, '$base_present' -- string `true` or `false`. + +diff_cmd:: + should actually launch the tool in diffing mode for a single path. + Positional argument: '$1' -- name of the diff tool. Predefined + variables: '$LOCAL' and '$REMOTE', '$merge_tool_path' -- absolute path + to the binary of the tool or its name if seen in default search path. + +diff_combo_supported:: + returns zero status (true) if the tool can operate in tabbed mode, + otherwise the feature will be skipped. Default: false. + +diff_combo_cmd:: + should actually launch the tool with all compared files. The function + receives list of pairs of files to compare, every filename is followed + by proper input field separator, '$IFS', the list is reachable through + third opened file descriptor and the function may close this fd after + reading all its content. System file descriptors remain untouched. + Predefined variable: '$merge_tool_path' -- absolute path to the binary + of the tool or its name if seen in default search path. ++ +Example: ++ +[listing] +diff_combo_cmd () { + "$merge_tool_path" -- `cat <&3` 3<&- +} + +translate_merge_tool_path:: + can print preset '$merge_tool_path'. Positional argument: '$1' -- name + of the merge tool. + +exit_code_trustable:: + returns zero status (true) if a caller is allowed to rely on the exit + code of the merge tool, otherwise one should check a backup of the file + to determine whether or not merging was successful. Default: false. + +list_tool_variants:: + prints all possible names that may be passed to the '--tool' option. + GIT --- Part of the linkgit:git[1] suite diff --git a/Documentation/git.txt b/Documentation/git.txt index a6d4ad0818..3ef75aae36 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -583,6 +583,14 @@ For each path `GIT_EXTERNAL_DIFF` is called, two environment variables, `GIT_DIFF_PATH_TOTAL`:: The total number of paths. +`GIT_DIFFTOOL_PROMPT`:: + Issue an interactive prompting right before launch the diff + tool. It must contain a boolean value. + +`GIT_DIFFTOOL_TABBED`:: + Run the diff tool in tabbed mode opening all compared files + together. It must contain a boolean value. + other ~~~~~ `GIT_MERGE_VERBOSITY`:: -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/4] t7800: new tests of difftool.tabbed feature 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev ` (2 preceding siblings ...) 2021-01-25 21:21 ` [PATCH v3 3/4] doc: describe new difftool.tabbed feature Nicholas Guriev @ 2021-01-25 21:21 ` Nicholas Guriev 3 siblings, 0 replies; 14+ messages in thread From: Nicholas Guriev @ 2021-01-25 21:21 UTC (permalink / raw) To: git A few helper functions were put in the script that manage faked binaries in a temporary directory for $PATH. The helpers restore state before test finish. Besides, two tests of rewritten prompting were added. "git difftool" now asks again on incorrect input. Plus, fixed a typo in a test of the --extcmd option. Signed-off-by: Nicholas Guriev <nicholas@guriev.su> --- t/t7800-difftool.sh | 175 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index a578b35761..249d7a4821 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -270,6 +270,22 @@ test_expect_success 'difftool last flag wins' ' prompt_given "$prompt" ' +test_expect_success 'ignore unknown input then launch the tool' ' + difftool_test_setup && + (echo Qwerty && echo Yes) >input && + git difftool branch <input >output && + tail -1 output | grep -q -x -F \ + "Launch '\''test-tool'\'' [Y/n]? Launch '\''test-tool'\'' [Y/n]? branch" +' + +test_expect_success 'ignore unknown input then skip the tool' ' + difftool_test_setup && + (echo Qwerty && echo No) >input && + git difftool branch <input >output && + tail -1 output | grep -q -x -F \ + "Launch '\''test-tool'\'' [Y/n]? Launch '\''test-tool'\'' [Y/n]? " +' + # git-difftool falls back to git-mergetool config variables # so test that behavior here test_expect_success 'difftool + mergetool config variables' ' @@ -319,7 +335,7 @@ test_expect_success 'difftool --extcmd=cat' ' test_expect_success 'difftool --extcmd cat' ' echo branch >expect && echo master >>expect && - git difftool --no-prompt --extcmd=cat branch >actual && + git difftool --no-prompt --extcmd cat branch >actual && test_cmp expect actual ' @@ -390,6 +406,163 @@ test_expect_success 'ending prompt input with EOF' ' ! grep br2 output ' +prepend_shared_path () { + directory="$1" + first="${PATH%%:*}" + if test "$directory" != "$first" + then + : ${clean_shared_path=$PATH} + PATH="$directory:$PATH" + test_when_finished 'PATH=$clean_shared_path' + fi +} + +mock_binary () { + name="$1" body="$2" + prepend_shared_path "$PWD/fake-bin" && + touch run-cmds && + mkdir -p fake-bin && + write_script "fake-bin/$name" <<-EOF && + printf '%s ' $name "\$@" | + tr -s '[:space:]' ' ' | + sed 's/ \$/\\n/' >>"\$HOME/run-cmds" && + ${body:-true} + EOF + test_when_finished "rm -f fake-bin/$name run-cmds" +} + +test_invoked () { + pattern="$1" + if ! grep -q -x -e "$pattern" run-cmds + then + set -- $pattern + echo "$1 does not seem to be invoked" + echo "entry '$pattern' was not found in the 'run-cmds' file" + false + fi +} + +test_expect_success 'tabbed mode with --tabbed option' ' + mock_binary vim && + test_config_global diff.tool vimdiff && + yes | git difftool --tabbed branch | tee actual && + ( + printf "Viewing 2 files:\n" && + printf " '\''file'\''\n" && + printf " '\''file2'\''\n" && + printf "Launch '\''vimdiff'\'' [Y/n]? " + ) >expect && + test_cmp expect actual && + test_invoked "vim -R -f -c .* file .* file2" +' + +test_expect_success 'tabbed mode with GIT_DIFFTOOL_TABBED environment' ' + mock_binary vim && + test_config_global diff.tool vimdiff && + yes | GIT_DIFFTOOL_TABBED=yes git difftool branch | tee actual && + ( + printf "Viewing 2 files:\n" && + printf " '\''file'\''\n" && + printf " '\''file2'\''\n" && + printf "Launch '\''vimdiff'\'' [Y/n]? " + ) >expect && + test_cmp expect actual && + test_invoked "vim -R -f -c .* file .* file2" +' + +test_expect_success 'tabbed mode with difftool.tabbed setting' ' + mock_binary vim && + test_config_global diff.tool vimdiff && + test_config_global difftool.tabbed 1 && + yes | git difftool branch | tee actual && + ( + printf "Viewing 2 files:\n" && + printf " '\''file'\''\n" && + printf " '\''file2'\''\n" && + printf "Launch '\''vimdiff'\'' [Y/n]? " + ) >expect && + test_cmp expect actual && + test_invoked "vim -R -f -c .* file .* file2" +' + +test_expect_success 'environment variable wins over config in tabbed mode' ' + mock_binary vim && + test_config_global diff.tool vimdiff && + test_config_global difftool.tabbed true && + GIT_DIFFTOOL_TABBED=FALSE git difftool -y branch </dev/null >output && + test_must_be_empty output && + test_invoked "vim -R -f -d -c .* file" && + test_invoked "vim -R -f -d -c .* file2" +' + +test_expect_success 'cli option wins over environment in tabbed mode' ' + mock_binary vim && + test_config_global diff.tool vimdiff && + GIT_DIFFTOOL_TABBED=1 git difftool -y --no-tabbed branch </dev/null >output && + test_must_be_empty output && + test_invoked "vim -R -f -d -c .* file" && + test_invoked "vim -R -f -d -c .* file2" +' + +test_expect_success 'say no in tabbed mode' ' + mock_binary meld && + yes no | git difftool -t meld --tabbed branch && + ! test_invoked "meld\\>.*" +' + +test_expect_success 'no tabbed mode for single file' ' + mock_binary meld && + git difftool -y -t meld --tabbed branch file && + test_invoked "meld \\S\\+ file" +' + +test_expect_success 'both --tabbed and --trust-exit-code options' ' + mock_binary meld false && + test_config_global diff.tool meld && + test_config_global difftool.prompt false && + test_must_fail git difftool --tabbed --trust-exit-code branch >output && + test_must_be_empty output && + test_invoked "meld --diff \\S\\+ file --diff \\S\\+ file2" +' + +test_expect_success 'tempdir is still clean after successful tabbed mode' ' + mock_binary meld && + mkdir tempdir && + test_when_finished "rm -r tempdir" && + TMPDIR="$PWD/tempdir" git difftool -y -t meld --tabbed branch && + test_dir_is_empty tempdir +' + +test_expect_success 'tempdir is still clean after failed tabbed mode' ' + mock_binary meld false && + mkdir tempdir && + test_when_finished "rm -r tempdir" && + TMPDIR="$PWD/tempdir" git difftool -y -t meld --tabbed branch && + test_dir_is_empty tempdir +' + +test_background () { + # https://stackoverflow.com/a/45112755/5000805 + set -m + "$@" & + set +m +} + +# Create a named queue for synchronizing. Our test process will get blocked on +# "echo line" until faked meld reaches "cat chain", and so we do not kill early. +test_expect_success PIPE 'tempdir is still clean after SIGTERM in tabbed mode' ' + mkfifo chan && + mock_binary meld "while true; do cat chan; done" && + mkdir tempdir && + test_when_finished "rm -r tempdir" && + test_background env TMPDIR="$PWD/tempdir" \ + git difftool -y -t meld --tabbed branch && + echo line >chan && + kill -TERM -$! && + wait && # for clean up + test_dir_is_empty tempdir +' + test_expect_success 'difftool --tool-help' ' git difftool --tool-help >output && grep tool output -- 2.27.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] mergetools: support difftool.tabbed setting 2021-01-13 5:59 [RFC PATCH] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev @ 2021-02-12 5:51 ` David Aguilar 2021-02-12 22:21 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: David Aguilar @ 2021-02-12 5:51 UTC (permalink / raw) To: Nicholas Guriev; +Cc: Git Mailing List On Tue, Jan 12, 2021 at 10:07 PM Nicholas Guriev <nicholas@guriev.su> wrote: > > I was asked how to configure "git difftool" to open files using several > tabs and stop spawning diff application on every modified file. I looked > into Git source and found no possibility to run diff tool at one step. > > The patch allows a user to view diffs in single window at one go. The > current implementation is still poor and it can be used solely for > demonstration purposes. To see it in action, tweak the local gitconfig: > > git config difftool.prompt false > git config difftool.tabbed true > > Then run: > > git difftool -t vimdiff > > Or: > > git difftool -t meld > > The solution has some restrictions, diffing up to ten files works now (I > did not bother with dynamic memory allocation), and it does not handle spaces > in file names (I do not know how to pass them correctly to underlying tools > without "xargs -0"). > > I think the git-difftool--helper should be changed so that it could > process many files in single invocation and it would not use a temporary > file by itself. A similar behaviour can be done in git-mergetool, too. > > Do you have ideas how to better implement such a feature? Any comments > are welcome. > > P.S.: I'm attaching screenshots for a clear demo what I mean. > --- > diff.c | 4 ++-- > git-mergetool--lib.sh | 36 +++++++++++++++++++++++++++++++++++- > mergetools/meld | 4 ++++ > mergetools/vimdiff | 17 +++++++++++++++++ > 4 files changed, 58 insertions(+), 3 deletions(-) > > > I'm not really sure if "tabbed" is the best name for what's going on, though. It's really more of a "diff everything in one shot" mode, and it just so happens that the tools in question use tabs. General note -- similar to the convention followed by mergetool.hideResolved and other difftool things I think it would make sense for tools to be able to override this on a per-tool basis. That said, I wonder whether we need this new feature, or whether we should instead improve an existing one. I'm leaning towards improving the existing dir-diff feature as a better alternative. It's unfortunate that the "git difftool --dir-diff" feature doesn't seem to mesh well with vimdiff. It does work well with other tools that support diffing arbitrary directories, notably meld, xxdiff, etc. Regarding vimdiff + git difftool -d, there is this advice: https://stackoverflow.com/questions/8156493/git-vimdiff-and-dirdiff meld works just fine with "git difftool -d" (arguably nicer, since it gives you a directory view and a diff view in separate tabs), so if the only improvement is for vimdiff, then maybe the advice with the DirDiff plugin might be a better way to go. Having something like a "difftool.vimdiff.useDirDiff" configuration variable could be a way for us to adopt the advice that they offer there. We could have the dir-diff difftool mode set a variable that the vimdiff scriptlet could use to detect that we're in dir-diff mode. Then, when that variable is set, vimdiff could use, vim -f '+next' '+execute \"DirDiff\" argv(0) argv(1)' $LOCAL $REMOTE (as mentioned in the SO page) to invoke dir-diff mode in vim. That way the user only needs to set: git config difftool.vimdiff.useDirDiff ... and then "git diffftool -d" will integrate with the DirDiff plugin. What do you think about improving the vimdiff scriptlet to better integrate with "git difftool -d" instead? -- David ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] mergetools: support difftool.tabbed setting 2021-02-12 5:51 ` [RFC PATCH] mergetools: support difftool.tabbed setting David Aguilar @ 2021-02-12 22:21 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-02-12 22:21 UTC (permalink / raw) To: David Aguilar; +Cc: Nicholas Guriev, Git Mailing List David Aguilar <davvid@gmail.com> writes: > I'm not really sure if "tabbed" is the best name for what's going on, > though. It's really more of a "diff everything in one shot" mode, and > it just so happens that the tools in question use tabs. This statement matches my reaction to this new feature exactly. The way the external commands are triggered via GIT_EXTERNAL_DIFF mechanism makes it "easy" to show changes for one path at a time and "hard" to do so for all paths at once, but the resulting end-user experience that is forced to view one path at a time may be awkward. > That said, I wonder whether we need this new feature, or whether we > should instead improve an existing one. I'm leaning towards improving > the existing dir-diff feature as a better alternative. ... As a non-user, I have no strong opinion on the "new feature"; other than that I trust your judgement on the "difftool" design issues, that is. Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-02-12 22:26 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-13 5:59 [RFC PATCH] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-18 23:25 ` Junio C Hamano 2021-01-18 21:00 ` [RFC PATCH v2 2/3] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev 2021-01-18 21:00 ` [RFC PATCH v2 3/3] doc: describe new difftool.tabbed feature Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 1/4] mergetools: support difftool.tabbed setting Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev 2021-01-25 23:04 ` Junio C Hamano 2021-01-25 21:21 ` [PATCH v3 3/4] doc: describe new difftool.tabbed feature Nicholas Guriev 2021-01-25 21:21 ` [PATCH v3 4/4] t7800: new tests of " Nicholas Guriev 2021-02-12 5:51 ` [RFC PATCH] mergetools: support difftool.tabbed setting David Aguilar 2021-02-12 22:21 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).