{"thread":{"id":"37695","subject":"[PATCH] diff: Pass 'changes found' flag to external diff tools","startedAt":"2014-10-09T03:17:50Z","lastAt":"2014-10-09T03:17:50Z","messageCount":1,"participants":["Zoltan Klinger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250408","messageId":"1412824670-81736-1-git-send-email-zoltan.klinger@gmail.com","threadId":"37695","inReplyTo":null,"subject":"[PATCH] diff: Pass 'changes found' flag to external diff tools","fromName":"Zoltan Klinger","fromEmail":"zoltan.klinger@gmail.com","sentAt":"2014-10-09T03:17:50Z","receivedAt":"2014-10-09T03:17:50Z","isPatch":true,"sender":{"key":"zoltan.klinger@gmail.com","avatar":"https://avatars.githubusercontent.com/u/95923?v=4"},"body":"When GIT_EXTERNAL_DIFF is defined and git diff is called with any of the\n'ignore whitespace' type command line arguments such as -w, -b or\n--ignore-space-at-eol, git diff ignores these flags and invokes\nGIT_EXTERNAL_DIFF for each changed file in the diff queue. If the diff\nqueue contains file(s) with only whitespace changes, the external diff\ntool displays diffs for changes that the user specifically did not ask\nfor.\n\nFor external diff tools, therefore, it could be useful if git diff set a\nGIT_DIFF_FOUND_CHANGES environment variable for each file in the diff\nqueue. This variable could be examined by the external diff tool to\ndecide whether to show a diff or skip the file.\n\nTo implement it:\n(a) Modify diff.c to indicate to external diff tools which file\n    contains changes the tool should care about:\n        Only when GIT_EXTERNAL_DIFF is defined and either\n        --ignore-space-change, --ignore-all-space or\n        --ignore-space-at-eol option is set, run diffstat on the current\n        file in the diff queue to find out the number of deleted and\n        inserted changes. Based on this set the environment variable\n        GIT_DIFF_FOUND_CHANGES to \"1\" or \"0\". If GIT_EXTERNAL_DIFF is\n        defined but none of the 'ignore whitespace' command line\n        arguments is set, set GIT_DIFF_FOUND_CHANGES to \"1\".\n\n(b) Modify git-difftool--helper.sh to make use of the GIT_DIFF_FOUND_CHANGES\n    variable: do not invok the external diff tool on files for which\n    GIT_DIFF_FOUND_CHANGES is set to false.\n\n(c) Update documentation and create unit tests.\n\nSigned-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>\n---\n Documentation/git.txt    | 11 +++++++++--\n diff.c                   | 12 ++++++++++++\n git-difftool--helper.sh  | 39 ++++++++++++++++++++++++---------------\n t/t4020-diff-external.sh | 33 +++++++++++++++++++++++++++++++++\n 4 files changed, 78 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex c6175d4..46030d0 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -846,8 +846,9 @@ temporary file --- it is removed when 'GIT_EXTERNAL_DIFF' exits.\n For a path that is unmerged, 'GIT_EXTERNAL_DIFF' is called with 1\n parameter, <path>.\n +\n-For each path 'GIT_EXTERNAL_DIFF' is called, two environment variables,\n-'GIT_DIFF_PATH_COUNTER' and 'GIT_DIFF_PATH_TOTAL' are set.\n+For each path 'GIT_EXTERNAL_DIFF' is called, three environment\n+variables, 'GIT_DIFF_PATH_COUNTER', 'GIT_DIFF_PATH_TOTAL' and\n+'GIT_DIFF_FOUND_CHANGES' are set.\n \n 'GIT_DIFF_PATH_COUNTER'::\n \tA 1-based counter incremented by one for every path.\n@@ -855,6 +856,12 @@ For each path 'GIT_EXTERNAL_DIFF' is called, two environment variables,\n 'GIT_DIFF_PATH_TOTAL'::\n \tThe total number of paths.\n \n+'GIT_DIFF_FOUND_CHANGES'::\n+\tThis variable is set to \"0\" if 'GIT_EXTERNAL_DIFF' is called with\n+\teither --ignore-space-change, --ignore-all-space or\n+\t--ignore-space-at-eol argument and the path has only whitespace\n+\tchanges. Otherwise it is set to \"1\".\n+\n other\n ~~~~~\n 'GIT_MERGE_VERBOSITY'::\ndiff --git a/diff.c b/diff.c\nindex d7a5c81..a5045d5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2953,6 +2953,7 @@ static void run_external_diff(const char *pgm,\n \n \targv_array_pushf(&env, \"GIT_DIFF_PATH_COUNTER=%d\", ++o->diff_path_counter);\n \targv_array_pushf(&env, \"GIT_DIFF_PATH_TOTAL=%d\", q->nr);\n+\targv_array_pushf(&env, \"GIT_DIFF_FOUND_CHANGES=%d\", o->found_changes);\n \n \tif (run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env.argv))\n \t\tdie(_(\"external diff died, stopping at %s\"), name);\n@@ -3034,6 +3035,9 @@ static void fill_metainfo(struct strbuf *msg,\n \t}\n }\n \n+static void diff_flush_stat(struct diff_filepair *p, struct diff_options *o,\n+\t\t\t    struct diffstat_t *diffstat);\n+\n static void run_diff_cmd(const char *pgm,\n \t\t\t const char *name,\n \t\t\t const char *other,\n@@ -3067,6 +3071,14 @@ static void run_diff_cmd(const char *pgm,\n \t}\n \n \tif (pgm) {\n+\t\to->found_changes = 1;\n+\t\tif (DIFF_OPT_TST(o, DIFF_FROM_CONTENTS)) {\n+\t\t\tstruct diffstat_t diffstat;\n+\t\t\tmemset(&diffstat, 0, sizeof(struct diffstat_t));\n+\t\t\tdiff_flush_stat(p, o, &diffstat);\n+\t\t\to->found_changes = (diffstat.files[0]->added || diffstat.files[0]->deleted);\n+\t\t}\n+\n \t\trun_external_diff(pgm, name, other, one, two, xfrm_msg,\n \t\t\t\t  complete_rewrite, o);\n \t\treturn;\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 7ef36b9..1d3918c 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -40,27 +40,36 @@ launch_merge_tool () {\n \t# the user with the real $MERGED name before launching $merge_tool.\n \tif should_prompt\n \tthen\n-\t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n-\t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n-\t\tif use_ext_cmd\n+\t\tif test $GIT_DIFF_FOUND_CHANGES -eq 1\n \t\tthen\n-\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n-\t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n+\t\t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n+\t\t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n+\t\t\tif use_ext_cmd\n+\t\t\tthen\n+\t\t\t\tprintf \"Launch '%s' [Y/n]: \" \\\n+\t\t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n+\t\t\telse\n+\t\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n+\t\t\tfi\n+\t\t\tif read ans && test \"$ans\" = n\n+\t\t\tthen\n+\t\t\t\treturn\n+\t\t\tfi\n \t\telse\n-\t\t\tprintf \"Launch '%s' [Y/n]: \" \"$merge_tool\"\n-\t\tfi\n-\t\tif read ans && test \"$ans\" = n\n-\t\tthen\n-\t\t\treturn\n+\t\t\tprintf \"\\nSkipping (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n+\t\t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n \t\tfi\n \tfi\n \n-\tif use_ext_cmd\n+\tif test $GIT_DIFF_FOUND_CHANGES -eq 1\n \tthen\n-\t\texport BASE\n-\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n-\telse\n-\t\trun_merge_tool \"$merge_tool\"\n+\t\tif use_ext_cmd\n+\t\tthen\n+\t\t\texport BASE\n+\t\t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n+\t\telse\n+\t\t\trun_merge_tool \"$merge_tool\"\n+\t\tfi\n \tfi\n }\n \ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 0446201..09452f2 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -206,6 +206,39 @@ test_expect_success 'GIT_EXTERNAL_DIFF path counter/total' '\n \ttest_cmp expect counter.txt\n '\n \n+test_expect_success 'GIT_EXTERNAL_DIFF found changes' '\n+\techo the quick brown fox >whitespace.txt &&\n+\tgit add whitespace.txt &&\n+\techo \"  the    quick    brown    fox  \" >whitespace.txt &&\n+\twrite_script found_changes.sh <<-\\EOF &&\n+\techo $GIT_DIFF_FOUND_CHANGES >>changes.txt\n+\tEOF\n+\t>changes.txt &&\n+\tcat >expect <<-\\EOF &&\n+\t1\n+\t1\n+\t1\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=./found_changes.sh git diff &&\n+\ttest_cmp expect changes.txt\n+'\n+\n+test_expect_success 'GIT_EXTERNAL_DIFF ignore whitespace changes' '\n+\tcat >expect <<-\\EOF &&\n+\t1\n+\t1\n+\t0\n+\tEOF\n+\trm changes.txt &&\n+\tGIT_EXTERNAL_DIFF=./found_changes.sh git diff --ignore-all-space &&\n+\ttest_cmp expect changes.txt\n+'\n+\n+test_expect_success 'clean up whitespace leftovers' '\n+\tgit update-index --force-remove whitespace.txt &&\n+\trm whitespace.txt\n+'\n+\n test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n \ttouch file.ext &&\n \tgit add file.ext &&\n-- \n2.1.1\n"}]}