{"thread":{"id":"54820","subject":"Unexpected behavior on diff -I<regex> --name-only","startedAt":"2020-12-14T19:02:32Z","lastAt":"2020-12-21T19:30:43Z","messageCount":7,"participants":["Johannes Altmanninger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"412187","messageId":"20201214190054.lrdllbpf6jfrz573@gmail.com","threadId":"54820","inReplyTo":null,"subject":"Unexpected behavior on diff -I<regex> --name-only","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2020-12-14T19:00:54Z","receivedAt":"2020-12-14T19:02:32Z","isPatch":false,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"Since v2.28.0-2-g296d4a94e7 (diff: add -I<regex> that ignores matching changes)\ndiff -I<regex> can be used to suppress hunks of only matching lines.\nThis interacts in a surprising way with --name-only, which lists\nall changed files, regardless of whether they are filtered out by -I<regex>:\n\n\tgit diff HEAD~ -I ''\t\t\t# always empty\n\tgit diff HEAD~ -I '' --name-only\t# not empty, \"-I\" does nothing\n\nIt could be nice to only show names of files with matching hunks\n(or reject this combination of options?).\n"},{"id":"412194","messageId":"xmqqeejsdv7x.fsf@gitster.c.googlers.com","threadId":"54820","inReplyTo":"20201214190054.lrdllbpf6jfrz573@gmail.com","subject":"Re: Unexpected behavior on diff -I<regex> --name-only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-14T19:49:06Z","receivedAt":"2020-12-14T19:50:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Altmanninger <aclopte@gmail.com> writes:\n\n> Since v2.28.0-2-g296d4a94e7 (diff: add -I<regex> that ignores matching changes)\n> diff -I<regex> can be used to suppress hunks of only matching lines.\n> This interacts in a surprising way with --name-only, which lists\n> all changed files, regardless of whether they are filtered out by -I<regex>:\n>\n> \tgit diff HEAD~ -I ''\t\t\t# always empty\n> \tgit diff HEAD~ -I '' --name-only\t# not empty, \"-I\" does nothing\n>\n> It could be nice to only show names of files with matching hunks\n> (or reject this combination of options?).\n\nInteresting.  \n\nThis is not a new issue limited to -I at all.  If you did this:\n\n\t$ echo \"hello\" >world\n\t$ git add world ; git commit -m 'add world'\n\t$ echo \" hello\" >world ; git add world\n\t$ git diff -w --cached\n\t$ git diff --name-only --cached\n\tworld\n\t$ git diff --name-only -w --cached\n\tworld\n\nI think \"--name-only\", and perhaps other options, has too aggressive\nan optimization that takes advantage of the fact that we can tell if\na path has changed or not without looking at the contents at all by\nlooking at the object name recorded.  That optimization may have\nbeen valid until many newer and more expensive features came around,\nbut not anymore.\n\nI think diff.c::flush_one_pair() needs to learn to pay attention to\nopt->diff_from_contents in its third branch where DIFF_FORMAT_NAME\nis handled.  I do not offhand remember if -I flips diff_from_contents\nbit, but I wouldn't be surprised if the recent change added the\nsupport for -I forgot to do so.\n\nThanks.\n\n\n\n\n\n\n\n\n\n"},{"id":"412447","messageId":"20201216231840.3163806-1-aclopte@gmail.com","threadId":"54820","inReplyTo":"xmqqeejsdv7x.fsf@gitster.c.googlers.com","subject":"Re: Unexpected behavior on diff -I<regex> --name-only","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2020-12-16T23:18:39Z","receivedAt":"2020-12-16T23:20:05Z","isPatch":false,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"\nOn Mon, Dec 14, 2020 at 11:49:06AM -0800, Junio C Hamano wrote:\n> This is not a new issue limited to -I at all.  If you did this:\nRight, I didn't think of -w at all!\n\n> I do not offhand remember if -I flips diff_from_contents\n> bit, but I wouldn't be surprised if the recent change added the\n> support for -I forgot to do so.\n\nSpot on ;)\n\n"},{"id":"412448","messageId":"20201216231840.3163806-2-aclopte@gmail.com","threadId":"54820","inReplyTo":"20201216231840.3163806-1-aclopte@gmail.com","subject":"[PATCH] diff: suppress --name-only paths where all hunks are ignored","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2020-12-16T23:18:40Z","receivedAt":"2020-12-16T23:20:06Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"Diff options -w and the new -I<regex> can be used to suppress some hunks. Honor\nthese ignores in combination with --name-only, --name-stat, and --raw,\nto not output files where all hunks are ignored.\n\nCommit f245194f9a made \"git diff -w --exit-code\" exit with zero if all\nchanged hunks are whitespace. This uses the diff_from_contents bit.\nSet that also when given -I<regex>, for consistent exit codes.\n\nThe diff_from_contents bit means that we have to look at content\nchanges to know if a path has changed - modulo ignored hunks.  Teach\ndiff.c::flush_one_pair() to do so.  In the caller, reset the found_changes bit\nafter each file pair, so we can test each file separately for content changes.\n\nSigned-off-by: Johannes Altmanninger <aclopte@gmail.com>\n---\n diff.c                     | 36 +++++++++++++++++++++++++++++++-----\n t/t4013-diff-various.sh    | 13 +++++++++++++\n t/t4015-diff-whitespace.sh |  9 +++++++++\n 3 files changed, 53 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 643f4f3f6d..560f2d5fad 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4630,11 +4630,10 @@ void diff_setup_done(struct diff_options *options)\n \t/*\n \t * Most of the time we can say \"there are changes\"\n \t * only by checking if there are changed paths, but\n-\t * --ignore-whitespace* options force us to look\n-\t * inside contents.\n+\t * --ignore-* options force us to look inside contents.\n \t */\n \n-\tif ((options->xdl_opts & XDF_WHITESPACE_FLAGS))\n+\tif ((options->xdl_opts & XDF_WHITESPACE_FLAGS) || options->ignore_regex)\n \t\toptions->flags.diff_from_contents = 1;\n \telse\n \t\toptions->flags.diff_from_contents = 0;\n@@ -5967,6 +5966,26 @@ static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n {\n \tint fmt = opt->output_format;\n \n+\tif (opt->flags.diff_from_contents &&\n+\t    (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME | DIFF_FORMAT_NAME_STATUS))) {\n+\t\tstatic FILE *devnull;\n+\t\tFILE *diff_file;\n+\n+\t\tif (!devnull)\n+\t\t\tdevnull = xfopen(\"/dev/null\", \"w\");\n+\n+\t\tdiff_file = opt->file;\n+\t\topt->file = devnull;\n+\t\topt->color_moved = 0;\n+\n+\t\tif (check_pair_status(p))\n+\t\t\tdiff_flush_patch(p, opt);\n+\n+\t\topt->file = diff_file;\n+\t\tif (!opt->found_changes)\n+\t\t\treturn;\n+\t}\n+\n \tif (fmt & DIFF_FORMAT_CHECKDIFF)\n \t\tdiff_flush_checkdiff(p, opt);\n \telse if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))\n@@ -6350,11 +6369,18 @@ void diff_flush(struct diff_options *options)\n \t\t\t     DIFF_FORMAT_NAME |\n \t\t\t     DIFF_FORMAT_NAME_STATUS |\n \t\t\t     DIFF_FORMAT_CHECKDIFF)) {\n+\t\tint found_changes = 0;\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (check_pair_status(p))\n-\t\t\t\tflush_one_pair(p, options);\n+\t\t\tif (!check_pair_status(p))\n+\t\t\t\tcontinue;\n+\t\t\tflush_one_pair(p, options);\n+\t\t\tif (options->found_changes) {\n+\t\t\t\tfound_changes = 1;\n+\t\t\t\toptions->found_changes = 0;\n+\t\t\t}\n \t\t}\n+\t\toptions->found_changes = found_changes;\n \t\tseparator++;\n \t}\n \ndiff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh\nindex f72d456d3b..7cfd3a22d1 100755\n--- a/t/t4013-diff-various.sh\n+++ b/t/t4013-diff-various.sh\n@@ -509,6 +509,19 @@ test_expect_success 'diff -I<regex> --stat' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'diff -I<regex> --name-only' '\n+\tgit diff -I \"\" >actual --exit-code &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'diff -I<regex> --name-status' '\n+\t! git diff -I\"[0-9]\" --name-status  --exit-code >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tM\tfile0\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'diff -I<regex>: detect malformed regex' '\n \ttest_expect_code 129 git diff --ignore-matching-lines=\"^[124-9\" 2>error &&\n \ttest_i18ngrep \"invalid regex given to -I: \" error\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 47f0e2889d..3c4941cf96 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -805,6 +805,15 @@ test_expect_success 'whitespace-only changes not reported (diffstat)' '\n \ttest_must_be_empty actual\n '\n \n+test_expect_success 'whitespace-only changes not reported (name-only)' '\n+\t# reuse state from previous test\n+\t! git diff --name-only >actual --exit-code &&\n+\tgit diff --name-only -b >actual --exit-code &&\n+\tgit diff --name-status -b >>actual &&\n+\tgit diff --raw -b >>actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_expect_success 'whitespace changes with modification reported (diffstat)' '\n \tgit reset --hard &&\n \techo >x \"hello  world\" &&\n-- \n2.29.2\n\n"},{"id":"412469","messageId":"xmqq4kkl1atq.fsf@gitster.c.googlers.com","threadId":"54820","inReplyTo":"20201216231840.3163806-2-aclopte@gmail.com","subject":"Re* [PATCH] diff: suppress --name-only paths where all hunks are ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-17T01:27:13Z","receivedAt":"2020-12-17T01:28:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Altmanninger <aclopte@gmail.com> writes:\n\n> Diff options -w and the new -I<regex> can be used to suppress some hunks. Honor\n> these ignores in combination with --name-only, --name-stat, and --raw,\n> to not output files where all hunks are ignored.\n\nHmph, I am not sure if --raw should be affected by any of the\nwhitespace-difference suppression feature, though.\n\n> Commit f245194f9a made \"git diff -w --exit-code\" exit with zero if all\n> changed hunks are whitespace. This uses the diff_from_contents bit.\n> Set that also when given -I<regex>, for consistent exit codes.\n\nThis one I can agree with.\n\n> The diff_from_contents bit means that we have to look at content\n> changes to know if a path has changed - modulo ignored hunks.  \n\nI am not sure what you mean by \" - modulo ignored hunks\" here.  When\nthe diff_from_contents bit is not in effect, we can just rely on\nbyte-for-byte equality to determine if a path has changed, which\nmeans we can say that two blobs are different by seeing that they\nhave different object names.  But when diff_from_contents bit is in\neffect, two blobs that are not byte-for-byte equal could still be\nconsidered the same.\n\nBut your sentence without \" - modulo ignored hunks\" says that very\nclearly.  So perhaps these three words can just go away?\n\n> Teach\n> diff.c::flush_one_pair() to do so.  In the caller, reset the found_changes bit\n> after each file pair, so we can test each file separately for content changes.\n\nThis part I am not sure (and later I will become even less sure it\nis right).\n\nIt's not like we were buggy when diff_from_contents bit is in effect\nfor all output formats, is it?  How does this change interact with\nthe use of the found_changes bit in diffcore_flush() where NO_OUTPUT\nformat is given, the command wants to exit with status and\ndiff_from_contents is in effect?\n\n\n> @@ -5967,6 +5966,26 @@ static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n>  {\n>  \tint fmt = opt->output_format;\n>  \n> +\tif (opt->flags.diff_from_contents &&\n> +\t    (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME | DIFF_FORMAT_NAME_STATUS))) {\n> +\t\tstatic FILE *devnull;\n> +\t\tFILE *diff_file;\n> +\n> +\t\tif (!devnull)\n> +\t\t\tdevnull = xfopen(\"/dev/null\", \"w\");\n> +\n> +\t\tdiff_file = opt->file;\n> +\t\topt->file = devnull;\n> +\t\topt->color_moved = 0;\n\nWhy is color_moved so special?  If we added some other new option,\nhow would we make sure that the person who is adding that new option\nwould remember to reset it here?  And how does that person decide if\nhis or her new option needs resetting or not in the first place?\n\nIt almost feels backwards to me, to be honest.  All this rigmarole\ncomes because opt that is used for the 'main' diff is reused by the\ninner diff you run here.  You save away opt->file and swap in a new\nvalue, and restore it before you leave.  You disable color_moved but\nforget to restore it, which may be an indication of new bug.  If we\nused a brand new diff_options instance and copied the options that\nmattered from *opt to the new one, and used the new one to drive\nthis inner diff, at least we wouldn't have to worry about destroying\nthe outer 'opt' like this patch tries to avoid (and probably not\nvery successfully).\n\nAlso I am not sure if \"caching\" the file handle to /dev/null is\ngood idea or not.  When will it be closed?  Would repeated\ninvocation of the diff machinery work well with it (think: \"git log\n-w --name-only\")\n\n> +\t\tif (check_pair_status(p))\n> +\t\t\tdiff_flush_patch(p, opt);\n> +\n> +\t\topt->file = diff_file;\n> +\t\tif (!opt->found_changes)\n> +\t\t\treturn;\n\nThis somehow feels wrong.  For one thing, RAW is about showing\nobject names of preimage and postimage, which means it shows the\npreimage and postimage are either identical or different, and\noptions like -w, -I, etc. that inspect and hide some textual\ndifferences should not affect its outcome at all.\n\nThe body of this function (without the above addition) looks like\nthis:\n\n        static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n        {\n                int fmt = opt->output_format;\n\n                if (fmt & DIFF_FORMAT_CHECKDIFF)\n                        diff_flush_checkdiff(p, opt);\n                else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))\n                        diff_flush_raw(p, opt);\n                else if (fmt & DIFF_FORMAT_NAME) {\n                        const char *name_a, *name_b;\n                        name_a = p->two->path;\n                        name_b = NULL;\n                        strip_prefix(opt->prefix_length, &name_a, &name_b);\n                        fprintf(opt->file, \"%s\", diff_line_prefix(opt));\n                        write_name_quoted(name_a, opt->file, opt->line_termination);\n                }\n        }\n\nand we notice that FORMAT_NAME case is an oddball among the three\nwith open-coded logic here without any helper function.  I would\nhave expected that FORMAT_NAME would be the only thing that should\nbe affected, and the way to do so would be to move what we see here\nto a new diff_flush_name_only(p, opt) helper function, and have that\nfunction pay the overhead of actually running a textual diff when\nthe diff_from_contents bit is in effect.\n\nHaving said that, after seeing RAW and NAME_STATUS are grouped into\nthe same group, I am having a second thought.\n\n> @@ -6350,11 +6369,18 @@ void diff_flush(struct diff_options *options)\n>  \t\t\t     DIFF_FORMAT_NAME |\n>  \t\t\t     DIFF_FORMAT_NAME_STATUS |\n>  \t\t\t     DIFF_FORMAT_CHECKDIFF)) {\n> +\t\tint found_changes = 0;\n>  \t\tfor (i = 0; i < q->nr; i++) {\n>  \t\t\tstruct diff_filepair *p = q->queue[i];\n> -\t\t\tif (check_pair_status(p))\n> -\t\t\t\tflush_one_pair(p, options);\n> +\t\t\tif (!check_pair_status(p))\n> +\t\t\t\tcontinue;\n> +\t\t\tflush_one_pair(p, options);\n> +\t\t\tif (options->found_changes) {\n> +\t\t\t\tfound_changes = 1;\n> +\t\t\t\toptions->found_changes = 0;\n> +\t\t\t}\n>  \t\t}\n> +\t\toptions->found_changes = found_changes;\n>  \t\tseparator++;\n>  \t}\n\nSee above---this seems to be an unwanted side effect of an\nunnecessary reuse of opt in flush_one_pair().\n\nHow about turning this into at least two patches?\n\n - We are not handling \"-I\" the same way as \"-w\", which is one\n   issue.  I think I can agree with the patch to fix that bug.\n\n - And then there is an issue that we are not inspecting the\n   contents when \"--name-only\" and \"-w\" is given together.  It is a\n   separate issue, and I am not even sure if it is a bug.  The\n   attempted \"fix\" we see here does not look so clean, either, and I\n   am tempted to declare that just like \"raw\", \"name-only\" and\n   \"name-status\" formats work with byte-for-byte equality and \"-w\"\n   and friends are ignored just like in \"diff --name-status --patch\"\n   the \"--patch\" option is ignored.\n\nIn any case, the first half is a lot more straightforward and it is\neasy to convince any reader of its correctness.\n\nThanks.\n\n--- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---\n\nSubject: [PATCH] diff: correct interaction between --exit-code and -I<pattern>\n\nJust like \"git diff -w --exit-code\" should exit with 0 when ignoring\nwhitespace differences results in no changes shown, if ignoring\ncertain changes with \"git diff -I<pattern> --exit-code\" result in an\nempty patch, we should exit with 0.\n\nThe test suite did not cover the interaction between \"--exit-code\"\nand \"-w\"; add one while adding a new test for \"--exit-code\" + \"-I\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                     |  3 ++-\n t/t4015-diff-whitespace.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 9768d8eab4..69e3bc00ed 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4637,7 +4637,8 @@ void diff_setup_done(struct diff_options *options)\n \t * inside contents.\n \t */\n \n-\tif ((options->xdl_opts & XDF_WHITESPACE_FLAGS))\n+\tif ((options->xdl_opts & XDF_WHITESPACE_FLAGS) ||\n+\t    options->ignore_regex_nr)\n \t\toptions->flags.diff_from_contents = 1;\n \telse\n \t\toptions->flags.diff_from_contents = 0;\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 47f0e2889d..8c574221b2 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -567,6 +567,30 @@ test_expect_success '--check and --quiet are not exclusive' '\n \tgit diff --check --quiet\n '\n \n+test_expect_success '-w and --exit-code interact sensibly' '\n+\ttest_when_finished \"git checkout x\" &&\n+\t{\n+\t\ttest_seq 15 &&\n+\t\techo \" 16\"\n+\t} >x &&\n+\ttest_must_fail git diff --exit-code &&\n+\tgit diff -w >actual &&\n+\ttest_must_be_empty actual &&\n+\tgit diff -w --exit-code\n+'\n+\n+test_expect_success '-I and --exit-code interact sensibly' '\n+\ttest_when_finished \"git checkout x\" &&\n+\t{\n+\t\ttest_seq 15 &&\n+\t\techo \" 16\"\n+\t} >x &&\n+\ttest_must_fail git diff --exit-code &&\n+\tgit diff -I. >actual &&\n+\ttest_must_be_empty actual &&\n+\tgit diff -I. --exit-code\n+'\n+\n test_expect_success 'check staged with no whitespace errors' '\n \techo \"foo();\" >x &&\n \tgit add x &&\n-- \n2.30.0-rc0-217-gff80b81bc6\n\n"},{"id":"412699","messageId":"20201220223435.tmo5ty5tzwu7et4d@gmail.com","threadId":"54820","inReplyTo":"xmqq4kkl1atq.fsf@gitster.c.googlers.com","subject":"Re* [PATCH] diff: suppress --name-only paths where all hunks are ignored","fromName":"Johannes Altmanninger","fromEmail":"aclopte@gmail.com","sentAt":"2020-12-20T22:34:35Z","receivedAt":"2020-12-21T05:43:18Z","isPatch":true,"sender":{"key":"aclopte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6853872?v=4"},"body":"On Wed, Dec 16, 2020 at 05:27:13PM -0800, Junio C Hamano wrote:\n> Johannes Altmanninger <aclopte@gmail.com> writes:\n> > The diff_from_contents bit means that we have to look at content\n> > changes to know if a path has changed - modulo ignored hunks.  \n>\n> But your sentence without \" - modulo ignored hunks\" says that very\n> clearly.  So perhaps these three words can just go away?\n\nMy bad, this should rather be\n\n\tThe diff_from_contents bit means that we have to look at content\n\tchanges (modulo ignored hunks) to know if a path has changed.\n\nprobably dropping the three words makes it even less confusing.\n\n> It's not like we were buggy when diff_from_contents bit is in effect\n> for all output formats, is it?\n\nRight, it's useless to set the bit here.\n\n> > +\t\topt->color_moved = 0;\n> \n> Why is color_moved so special?  If we added some other new option,\n> how would we make sure that the person who is adding that new option\n> would remember to reset it here?  And how does that person decide if\n> his or her new option needs resetting or not in the first place?\n\nI copied color_moved=0 from the DIFF_FORMAT_NO_OUTPUT section of\ndiff_flush(). It happens to work correctly in both places because no diff\ncontents are printed, but yeah it's not robust to future changes.\n\n> Also I am not sure if \"caching\" the file handle to /dev/null is\n> good idea or not.  When will it be closed?  Would repeated\n> invocation of the diff machinery work well with it (think: \"git log\n> -w --name-only\")\n\nI believe that repeated invocations work fine, because the file is never\nclosed (which may be a problem?). The file could be made nilable if we still\nneed something like that.\n\n> This somehow feels wrong.  For one thing, RAW is about showing\n> object names of preimage and postimage, which means it shows the\n> preimage and postimage are either identical or different, and\n> options like -w, -I, etc. that inspect and hide some textual\n> differences should not affect its outcome at all.\n\nYeah, I realized late that -w --raw will have sort of unintuitive semantics.\nThis combination is probably rare but I guess users could set an alias for\ndiff -w.\n\n>    I am tempted to declare that just like \"raw\", \"name-only\" and\n>    \"name-status\" formats work with byte-for-byte equality\n\n\nOK, I think it's better to keep the current (consistent) behavior.  I don't\nthink it's worth to special-case \"--name-only\".\n\nIt's easy to read the names from a \"diff -I\" output anyway.  This filter is\nbroken in various ways, but works well enough in practise:\n\n\tdiff -I | perl -ne 'print \"$1\\n\" if /\\+{3} b\\/(.*)/'\n\n> and \"-w\" and friends are ignored just like in \"diff --name-status --patch\"\n> the \"--patch\" option is ignored.\n\nFor interactive use, it would be more helpful to reject useless options\ninstead of ignoring them. Though ignoring is probably desirable to allow\nliberal use of these options, I'm not sure.\n\n> --- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---\n> \n> Subject: [PATCH] diff: correct interaction between --exit-code and -I<pattern>\n\nLooks good.\n\n> +test_expect_success '-w and --exit-code interact sensibly' '\n\nMaybe 'exit with 0 when all changes are ignored by -w' though either version\nis fine because I think the intention of the test is already obvious.\n"},{"id":"412766","messageId":"xmqq1rfjlzz1.fsf@gitster.c.googlers.com","threadId":"54820","inReplyTo":"20201220223435.tmo5ty5tzwu7et4d@gmail.com","subject":"Re: Re* [PATCH] diff: suppress --name-only paths where all hunks are ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-21T19:29:38Z","receivedAt":"2020-12-21T19:30:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Altmanninger <aclopte@gmail.com> writes:\n\n>> +test_expect_success '-w and --exit-code interact sensibly' '\n>\n> Maybe 'exit with 0 when all changes are ignored by -w' though either version\n> is fine because I think the intention of the test is already obvious.\n\nYeah, 'sensibly' is a zero-bit phrase and the letters are better\nspent on describing what we deem sensible more clearly.  Thanks.\n"}]}