{"thread":{"id":"30817","subject":"[PATCH] diff: exit(1) if 'diff --quiet ' finds changes","startedAt":"2012-06-15T17:31:03Z","lastAt":"2012-06-18T18:00:31Z","messageCount":12,"participants":["Tim Henigan","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"193708","messageId":"1339781463-13536-1-git-send-email-tim.henigan@gmail.com","threadId":"30817","inReplyTo":null,"subject":"[PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-06-15T17:31:03Z","receivedAt":"2012-06-15T17:31:03Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"When running 'git diff --quiet ', if file1 or file2\nis outside the repository, it will exit(0) even if the files differ.\nIt should exit(1) when they differ.\n\nSigned-off-by: Tim Henigan \n---\n\nThis was the least invasive fix that I found. I considered adding\nthe following when the '--quiet' option is parsed instead:\n\n+ DIFF_OPT_SET(options, EXIT_WITH_STATUS)\n+ DIFF_OPT_SET(options, DIFF_FROM_CONTENTS)\n\n\n diff.c | 5 +++--\n t/t4035-diff-quiet.sh | 5 +++++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 77edd50..b1d74fe 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4432,9 +4432,10 @@ void diff_flush(struct diff_options *options)\n \t\tseparator++;\n \t}\n \n-\tif (output_format & DIFF_FORMAT_NO_OUTPUT &&\n+\tif ((output_format & DIFF_FORMAT_NO_OUTPUT &&\n \t DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&\n-\t DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {\n+\t DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) ||\n+\t\tDIFF_OPT_TST(options, QUICK)) {\n \t\t/*\n \t\t * run diff_flush_patch for the exit status. setting\n \t\t * options->file to /dev/null should be safe, becaue we\ndiff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh\nindex cdb9202..e14e676 100755\n--- a/t/t4035-diff-quiet.sh\n+++ b/t/t4035-diff-quiet.sh\n@@ -77,4 +77,9 @@ test_expect_success 'git diff-index --cached HEAD' '\n \t}\n '\n \n+test_expect_success 'git diff ' '\n+\tgit diff --quiet c /dev/null\n+\ttest $? = 1\n+'\n+\n test_done\n-- \n1.7.11.rc3.6.g501bf14\n"},{"id":"193719","messageId":"20120615184030.GC14843@sigill.intra.peff.net","threadId":"30817","inReplyTo":"1339781463-13536-1-git-send-email-tim.henigan@gmail.com","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-15T18:40:30Z","receivedAt":"2012-06-15T18:40:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 15, 2012 at 01:31:03PM -0400, Tim Henigan wrote:\n\n> When running 'git diff --quiet ', if file1 or file2\n> is outside the repository, it will exit(0) even if the files differ.\n> It should exit(1) when they differ.\n\n>From your description, I would expect the fix to be in builtin/diff.c,\nor in diff-no-index.c, since that is where the code paths diverge.\n\n> This was the least invasive fix that I found. I considered adding\n> the following when the '--quiet' option is parsed instead:\n> \n> + DIFF_OPT_SET(options, EXIT_WITH_STATUS)\n> + DIFF_OPT_SET(options, DIFF_FROM_CONTENTS)\n\nWe already set EXIT_WITH_STATUS when we see --quiet (we just do it a\nlittle later, during diff_setup_done). We would not want to set\nDIFF_FROM_CONTENTS all the time with --quiet. The point of that flag is\n\"we cannot know just from seeing the path sha1s whether they are\ndifferent or not, because we are doing content-level munging\" (for\nexample, things like ignoring whitespace changes).\n\nSo we would not want to always set it whenever --quiet is given, because\nit means we must do a lot of extra work comparing file content.\n\n> diff --git a/diff.c b/diff.c\n> index 77edd50..b1d74fe 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -4432,9 +4432,10 @@ void diff_flush(struct diff_options *options)\n> \t\tseparator++;\n> \t}\n> \n> -\tif (output_format & DIFF_FORMAT_NO_OUTPUT &&\n> +\tif ((output_format & DIFF_FORMAT_NO_OUTPUT &&\n> \t DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&\n> -\t DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {\n> +\t DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) ||\n> +\t\tDIFF_OPT_TST(options, QUICK)) {\n> \t\t/*\n> \t\t * run diff_flush_patch for the exit status. setting\n> \t\t * options->file to /dev/null should be safe, becaue we\n\nAnd this is equally bad, because it means that --quiet gets much slower\nfor _all_ cases, not just the no-index case.\n\nI suspect what you actually want is to set DIFF_FROM_CONTENTS in the\nno-index case, since we by definition do not have a pair of sha1s to\ncompare. But it may also be that diff.c could detect this case\nautomatically. I'd have to look closer.\n\n-Peff\n"},{"id":"193721","messageId":"7vzk849zxg.fsf@alter.siamese.dyndns.org","threadId":"30817","inReplyTo":"1339781463-13536-1-git-send-email-tim.henigan@gmail.com","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-15T18:45:47Z","receivedAt":"2012-06-15T18:45:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n> This was the least invasive fix that I found.\n\nI think this breaks a normal case of comparing revisions and tracked\ncontents in a big way.\n\nThe \"quick\" thing is meant to notice any difference in the paths\ninvolved (e.g. \"git diff maint master -- t/\") at the blob object\nname level without having to look at the contents when there is no\nDIFF_FROM_CONTENTS processing is needed. We call into the more\nexpensive diff_flush_patch() codepath only when things like \"ignore\nwhitespace change\" is given, in which case we would need to compare\nthe contents.\n\nYour patch seems to be making us go through diff_flush_patch()\ncodepath unconditionally, and it looks like it is only to sweep some\nother breakage in diff-no-index codepath under the rug.\n\nHave you looked at how \"git diff --quiet HEAD^\" (without any other\noption) for tracked files notices that there is a difference and\nexit with non-zero status? Is it doing something wrong? Otherwise\nwhy can't the no-index codepath do the same thing?\n\nI think the following may be a lot closer to the correct fix; I\ndidn't test many combinations of options with it, though.\n\n diff-no-index.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f0b0010..ed74e27 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -172,7 +172,7 @@ void diff_no_index(struct rev_info *revs,\n \t\t int argc, const char **argv,\n \t\t int nongit, const char *prefix)\n {\n-\tint i;\n+\tint i, result;\n \tint no_index = 0;\n \tunsigned options = 0;\n \n@@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,\n \t * The return code for --no-index imitates diff(1):\n \t * 0 = no changes, 1 = changes, else error\n \t */\n-\texit(revs->diffopt.found_changes);\n+\tresult = !!diff_result_code(&revs->diffopt, 0);\n+\texit(result);\n }\n"},{"id":"193723","messageId":"20120615191353.GA26473@sigill.intra.peff.net","threadId":"30817","inReplyTo":"20120615184030.GC14843@sigill.intra.peff.net","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-15T19:13:53Z","receivedAt":"2012-06-15T19:13:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 15, 2012 at 02:40:30PM -0400, Jeff King wrote:\n\n> I suspect what you actually want is to set DIFF_FROM_CONTENTS in the\n> no-index case, since we by definition do not have a pair of sha1s to\n> compare. But it may also be that diff.c could detect this case\n> automatically. I'd have to look closer.\n\nThis works for me:\n\ndiff --git a/diff.c b/diff.c\nindex 77edd50..27231d2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3228,7 +3228,8 @@ int diff_setup_done(struct diff_options *options)\n \n \tif (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||\n \t DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||\n-\t DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))\n+\t DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL) ||\n+\t DIFF_OPT_TST(options, NO_INDEX))\n \t\tDIFF_OPT_SET(options, DIFF_FROM_CONTENTS);\n \telse\n \t\tDIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);\n\nOne could also set DIFF_FROM_CONTENTS manually in diff_on_index, but\nnote that diff_setup_done unsets it (which seems odd; who would have set\nit in the first place if they did not want to override it? I think that\nelse clause can simply be dropped).\n\n-Peff\n\nPS Your test should probably be spelled with test_expect_code, like:\n\ndiff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh\nindex cdb9202..0b83235 100755\n--- a/t/t4035-diff-quiet.sh\n+++ b/t/t4035-diff-quiet.sh\n@@ -77,4 +77,8 @@ test_expect_success 'git diff-index --cached HEAD' '\n \t}\n '\n \n+test_expect_success 'git diff ' '\n+\ttest_expect_code 1 git diff --quiet c /dev/null\n+'\n+\n test_done\n"},{"id":"193726","messageId":"20120615193724.GB26473@sigill.intra.peff.net","threadId":"30817","inReplyTo":"7vzk849zxg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-15T19:37:24Z","receivedAt":"2012-06-15T19:37:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 15, 2012 at 11:45:47AM -0700, Junio C Hamano wrote:\n\n> I think the following may be a lot closer to the correct fix; I\n> didn't test many combinations of options with it, though.\n> \n> diff-no-index.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index f0b0010..ed74e27 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -172,7 +172,7 @@ void diff_no_index(struct rev_info *revs,\n> \t\t int argc, const char **argv,\n> \t\t int nongit, const char *prefix)\n> {\n> -\tint i;\n> +\tint i, result;\n> \tint no_index = 0;\n> \tunsigned options = 0;\n> \n> @@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,\n> \t * The return code for --no-index imitates diff(1):\n> \t * 0 = no changes, 1 = changes, else error\n> \t */\n> -\texit(revs->diffopt.found_changes);\n> +\tresult = !!diff_result_code(&revs->diffopt, 0);\n> +\texit(result);\n\nHmm. This works because is checks HAS_CHANGES instead of found_changes.\nI was somewhat surprised that the former works at all, but it is because\ndiffcore_std sets it if there is anything in diff_queued_diff. Which\nmakes sense; found_changes seems to be only about communicating changes\nup from builtin_diff to the rest of the diff code; nobody should be\nreading it outside of there.\n\nWhich left me to wonder how we tell when two files are actually the\nsame. The answer is that diffcore_skip_stat_unmatch actually loads and\nmemcmps each pair, removing ones that are identical. And diff_no_index\nalways sets skip_stat_unmatch, whether diff.autorefreshindex is on or\nnot.\n\nSo the patch I posted elsewhere in the thread is not right; we do not\nneed to do diff_flush_patch to actually compare, because the\nstat_unmatch code will have done everything we want (unless\nDIFF_FROM_CONTENTS really is set). And the bug is purely one of looking\nat the wrong output flag.\n\n-Peff\n"},{"id":"193728","messageId":"CAFoueth2Hfcv0p0SZmichi_6e5--SNkemrSsSivnU73bdFOB4g@mail.gmail.com","threadId":"30817","inReplyTo":"20120615193724.GB26473@sigill.intra.peff.net","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-06-15T19:56:11Z","receivedAt":"2012-06-15T19:56:11Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Fri, Jun 15, 2012 at 3:37 PM, Jeff King wrote:\n> On Fri, Jun 15, 2012 at 11:45:47AM -0700, Junio C Hamano wrote:\n>\n>> I think this breaks a normal case of comparing revisions and tracked\n>> contents in a big way.\n\nI didn't understand the ramifications of making the change where I\ndid. I appreciate you and Jeff taking the time to point out the\nproblem (and suggest better solutions).\n\n\n>> I think the following may be a lot closer to the correct fix; I\n>> didn't test many combinations of options with it, though.\n>>\n>>  diff-no-index.c | 5 +++--\n>>  1 file changed, 3 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/diff-no-index.c b/diff-no-index.c\n>> index f0b0010..ed74e27 100644\n>> --- a/diff-no-index.c\n>> +++ b/diff-no-index.c\n>> @@ -172,7 +172,7 @@ void diff_no_index(struct rev_info *revs,\n>>                  int argc, const char **argv,\n>>                  int nongit, const char *prefix)\n>>  {\n>> -     int i;\n>> +     int i, result;\n>>       int no_index = 0;\n>>       unsigned options = 0;\n>>\n>> @@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,\n>>        * The return code for --no-index imitates diff(1):\n>>        * 0 = no changes, 1 = changes, else error\n>>        */\n>> -     exit(revs->diffopt.found_changes);\n>> +     result = !!diff_result_code(&revs->diffopt, 0);\n>> +     exit(result);\n\nI assume the '!!' before 'diff_result_code' is a typo. Reverting my\nchanges and putting this in solves the problem.\n\n...\n\n> So the patch I posted elsewhere in the thread is not right; we do not\n> need to do diff_flush_patch to actually compare, because the\n> stat_unmatch code will have done everything we want (unless\n> DIFF_FROM_CONTENTS really is set). And the bug is purely one of looking\n> at the wrong output flag.\n\nI will send v2 with the change to 'diff-no-index.c' suggested by\nJunio. I will also include the 'test_expect_code' improvement\nsuggested by Jeff.\n"},{"id":"193730","messageId":"7vmx449w3j.fsf@alter.siamese.dyndns.org","threadId":"30817","inReplyTo":"CAFoueth2Hfcv0p0SZmichi_6e5--SNkemrSsSivnU73bdFOB4g@mail.gmail.com","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-15T20:08:32Z","receivedAt":"2012-06-15T20:08:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n>>> @@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,\n>>> * The return code for --no-index imitates diff(1):\n>>> * 0 = no changes, 1 = changes, else error\n>>> */\n>>> - exit(revs->diffopt.found_changes);\n>>> + result = !!diff_result_code(&revs->diffopt, 0);\n>>> + exit(result);\n>\n> I assume the '!!' before 'diff_result_code' is a typo.\n\nNot a typo. I meant to use that idiom to turn 0 or not into\nboolean, as diff_result_code() can return values other than 0 or 1.\n"},{"id":"193731","messageId":"7vipes9w0p.fsf@alter.siamese.dyndns.org","threadId":"30817","inReplyTo":"CAFoueth2Hfcv0p0SZmichi_6e5--SNkemrSsSivnU73bdFOB4g@mail.gmail.com","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-15T20:10:14Z","receivedAt":"2012-06-15T20:10:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n> I will send v2 with the change to 'diff-no-index.c' suggested by\n> Junio. I will also include the 'test_expect_code' improvement\n> suggested by Jeff.\n\nAlso the test script shouldn't be just testing a simplest case, I\nwould think.\n\nFor example, comparing two files with \"a b c\" and \"a b c\" in them\nwith \"--quiet\" should yield \"They are different!\" while running the\nsame comparison with \"--quiet -w\" should say \"They are the same!\",\nno?\n"},{"id":"193734","messageId":"20120615202441.GA12163@sigill.intra.peff.net","threadId":"30817","inReplyTo":"7vmx449w3j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-15T20:24:41Z","receivedAt":"2012-06-15T20:24:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 15, 2012 at 01:08:32PM -0700, Junio C Hamano wrote:\n\n> Tim Henigan writes:\n> \n> >>> @@ -273,5 +273,6 @@ void diff_no_index(struct rev_info *revs,\n> >>> * The return code for --no-index imitates diff(1):\n> >>> * 0 = no changes, 1 = changes, else error\n> >>> */\n> >>> - exit(revs->diffopt.found_changes);\n> >>> + result = !!diff_result_code(&revs->diffopt, 0);\n> >>> + exit(result);\n> >\n> > I assume the '!!' before 'diff_result_code' is a typo.\n> \n> Not a typo. I meant to use that idiom to turn 0 or not into\n> boolean, as diff_result_code() can return values other than 0 or 1.\n\nI wonder if that is a good idea, though. AFAICT, diff_result_code will\nonly return a different exit code if \"--check\" is used. If we pass along\nthe exit code fully, then:\n\n 1. If --check is not used, we will be diff(1)-compatible.\n\n 2. If --check is used, then we will not be compatible with diff(1) in\n our exit code. But diff(1) does not have --check in the first\n place, so there is no point in us trying to be a drop-in\n replacement.\n\n-Peff\n"},{"id":"193738","messageId":"7vaa049uex.fsf@alter.siamese.dyndns.org","threadId":"30817","inReplyTo":"20120615202441.GA12163@sigill.intra.peff.net","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-15T20:44:54Z","receivedAt":"2012-06-15T20:44:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> 2. If --check is used, then we will not be compatible with diff(1) in\n> our exit code. But diff(1) does not have --check in the first\n> place, so there is no point in us trying to be a drop-in\n> replacement.\n\nSounds sensible. Thanks.\n"},{"id":"193789","messageId":"CAFouetjkSQuY6pyFdiVsM5tUnEiFK6uWTqDO38uhwUjaDVre1w@mail.gmail.com","threadId":"30817","inReplyTo":"7vipes9w0p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-06-18T17:51:51Z","receivedAt":"2012-06-18T17:51:51Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"On Fri, Jun 15, 2012 at 4:10 PM, Junio C Hamano wrote:\n> Tim Henigan writes:\n>\n>> I will send v2 with the change to 'diff-no-index.c' suggested by\n>> Junio.  I will also include the 'test_expect_code' improvement\n>> suggested by Jeff.\n>\n> Also the test script shouldn't be just testing a simplest case, I\n> would think.\n>\n> For example, comparing two files with \"a b c\" and \"a  b c\" in them\n> with \"--quiet\" should yield \"They are different!\" while running the\n> same comparison with \"--quiet -w\" should say \"They are the same!\",\n> no?\n\nTo obtain full coverage, it seems we need to test:\n 1) git diff --quiet \n 2) git diff --quiet \n\nfor each of the following options:\n a) no additional options\n b) --ignore-space-at-eol\n c) --ignore-all-space\n\nThese seem like the only diff options that should be tested...am I\nmissing anything?\n\nBecause some of the tests require one directory that is a repo and one\ndirectory that is not, it seems best to create a new test file with\n$TEST_NO_CREATE_REPO set. Then the setup function can create both the\nrepo and the plain directory.\n\nSo, I am thinking of adding 't/t4054-diff-outside-repo.sh' that does\nthe following:\n\n 1) setup (includes a cd into the repo directory)\n 2) git diff --quiet, one outside repo, matching files\n 3) git diff --quiet, one outside repo, different files\n 4) git diff --quiet, one outside repo, ignore trailing whitespace\n 5) git diff --quiet, one outside repo, ignore all whitespace\n 6) git diff --quiet, both outside repo, matching files\n 7) git diff --quiet, both outside repo, different files\n 8) git diff --quiet, both outside repo, ignore trailing whitespace\n 9) git diff --quiet, both outside repo, ignore all whitespace\n 10) cleanup (cd back to $HOME test directory)\n\nDoes this sound reasonable?\n"},{"id":"193792","messageId":"7v1ulc4i0w.fsf@alter.siamese.dyndns.org","threadId":"30817","inReplyTo":"CAFouetjkSQuY6pyFdiVsM5tUnEiFK6uWTqDO38uhwUjaDVre1w@mail.gmail.com","subject":"Re: [PATCH] diff: exit(1) if 'diff --quiet ' finds changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-18T18:00:31Z","receivedAt":"2012-06-18T18:00:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n> To obtain full coverage, it seems we need to test:\n> 1) git diff --quiet \n> 2) git diff --quiet \n>\n> for each of the following options:\n> a) no additional options\n> b) --ignore-space-at-eol\n> c) --ignore-all-space\n>\n> These seem like the only diff options that should be tested...am I\n> missing anything?\n>\n> Because some of the tests require one directory that is a repo and one\n> directory that is not, it seems best to create a new test file with\n> $TEST_NO_CREATE_REPO set. Then the setup function can create both the\n> repo and the plain directory.\n\nIt is probably overkill to add a new test that only to add 3 (the\n\"two paths outside repo\" among the 6 = 2 * 3 combinations above).\nInside t4035, you could dig new directory two levels deep, chdir to\nit and set GIT_CEILING_DIRECTORY to its first level and run test,\nno? Something along these lines (the diff invoked would obviously\nbe not between foo and bar)...\n\n t/t4035-diff-quiet.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh\nindex cdb9202..e7065cc 100755\n--- a/t/t4035-diff-quiet.sh\n+++ b/t/t4035-diff-quiet.sh\n@@ -77,4 +77,16 @@ test_expect_success 'git diff-index --cached HEAD' '\n \t}\n '\n \n+test_expect_success 'git diff-no-index outside repository' '\n+\tmkdir -p not/here &&\n+\t(\n+\t\tcd not/here &&\n+\t\tGIT_CEILING_DIRECTORIES=$TRASH_DIRECTORY/not &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\techo foo >foo &&\n+\t\techo bar >bar &&\n+\t\tgit diff foo bar\n+\t)\n+'\n+\n test_done\n"}]}