{"thread":{"id":"47397","subject":"[PATCH] diff: add tests for --relative without optional prefix value","startedAt":"2017-12-07T19:11:20Z","lastAt":"2017-12-07T22:05:59Z","messageCount":7,"participants":["Jacob Keller","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334349","messageId":"20171207190135.28660-1-jacob.e.keller@intel.com","threadId":"47397","inReplyTo":null,"subject":"[PATCH] diff: add tests for --relative without optional prefix value","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-12-07T19:01:35Z","receivedAt":"2017-12-07T19:11:20Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nWe already have tests for --relative, but they currently only test when\na prefix has been provided. This fails to test the case where --relative\nby itself should use the current directory as the prefix.\n\nTeach the check_$type functions to take a directory argument to indicate\nwhich subdirectory to run the git commands in. Add a new test which uses\nthis to test --relative without a prefix value.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n t/t4045-diff-relative.sh | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex 3950f5034d31..7d68a6e2a536 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -13,6 +13,7 @@ test_expect_success 'setup' '\n '\n \n check_diff() {\n+dir=$1; shift\n expect=$1; shift\n cat >expected <<EOF\n diff --git a/$expect b/$expect\n@@ -24,50 +25,54 @@ index 0000000..25c05ef\n +other content\n EOF\n test_expect_success \"-p $*\" \"\n-\tgit diff -p $* HEAD^ >actual &&\n+\tgit -C '$dir' diff -p $* HEAD^ >actual &&\n \ttest_cmp expected actual\n \"\n }\n \n check_numstat() {\n+dir=$1; shift\n expect=$1; shift\n cat >expected <<EOF\n 1\t0\t$expect\n EOF\n test_expect_success \"--numstat $*\" \"\n \techo '1\t0\t$expect' >expected &&\n-\tgit diff --numstat $* HEAD^ >actual &&\n+\tgit -C '$dir' diff --numstat $* HEAD^ >actual &&\n \ttest_cmp expected actual\n \"\n }\n \n check_stat() {\n+dir=$1; shift\n expect=$1; shift\n cat >expected <<EOF\n  $expect | 1 +\n  1 file changed, 1 insertion(+)\n EOF\n test_expect_success \"--stat $*\" \"\n-\tgit diff --stat $* HEAD^ >actual &&\n+\tgit -C '$dir' diff --stat $* HEAD^ >actual &&\n \ttest_i18ncmp expected actual\n \"\n }\n \n check_raw() {\n+dir=$1; shift\n expect=$1; shift\n cat >expected <<EOF\n :000000 100644 0000000000000000000000000000000000000000 25c05ef3639d2d270e7fe765a67668f098092bc5 A\t$expect\n EOF\n test_expect_success \"--raw $*\" \"\n-\tgit diff --no-abbrev --raw $* HEAD^ >actual &&\n+\tgit -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&\n \ttest_cmp expected actual\n \"\n }\n \n for type in diff numstat stat raw; do\n-\tcheck_$type file2 --relative=subdir/\n-\tcheck_$type file2 --relative=subdir\n-\tcheck_$type dir/file2 --relative=sub\n+\tcheck_$type . file2 --relative=subdir/\n+\tcheck_$type . file2 --relative=subdir\n+\tcheck_$type . dir/file2 --relative=sub\n+\tcheck_$type subdir file2 --relative\n done\n \n test_done\n-- \n2.15.1.478.ga1e07cd25f8b\n\n"},{"id":"334350","messageId":"xmqq374me4a7.fsf@gitster.mtv.corp.google.com","threadId":"47397","inReplyTo":"20171207190135.28660-1-jacob.e.keller@intel.com","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-07T19:17:36Z","receivedAt":"2017-12-07T19:17:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n>  for type in diff numstat stat raw; do\n> -\tcheck_$type file2 --relative=subdir/\n> -\tcheck_$type file2 --relative=subdir\n> -\tcheck_$type dir/file2 --relative=sub\n> +\tcheck_$type . file2 --relative=subdir/\n> +\tcheck_$type . file2 --relative=subdir\n> +\tcheck_$type . dir/file2 --relative=sub\n> +\tcheck_$type subdir file2 --relative\n\nOK, I didn't think it would be sensible to unconditionally pass the\ndirectory and use \"-C .\" as a no-op.  It looks good.\n\nI think the new one should go before the dir/file2 test; all three\nearlier tests (including this new one) are about taking a patch\nrelative to subdir/ spelled in different ways, and the one about\ndir/file2 alone is different.\n"},{"id":"334357","messageId":"CA+P7+xom-R3JX1ghZLDciRX=5zsJs=ngCX9iM=Y8m8TN5TX_=g@mail.gmail.com","threadId":"47397","inReplyTo":"xmqq374me4a7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-12-07T19:54:02Z","receivedAt":"2017-12-07T19:54:29Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 7, 2017 at 11:17 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jacob Keller <jacob.e.keller@intel.com> writes:\n>\n>>  for type in diff numstat stat raw; do\n>> -     check_$type file2 --relative=subdir/\n>> -     check_$type file2 --relative=subdir\n>> -     check_$type dir/file2 --relative=sub\n>> +     check_$type . file2 --relative=subdir/\n>> +     check_$type . file2 --relative=subdir\n>> +     check_$type . dir/file2 --relative=sub\n>> +     check_$type subdir file2 --relative\n>\n> OK, I didn't think it would be sensible to unconditionally pass the\n> directory and use \"-C .\" as a no-op.  It looks good.\n>\n> I think the new one should go before the dir/file2 test; all three\n> earlier tests (including this new one) are about taking a patch\n> relative to subdir/ spelled in different ways, and the one about\n> dir/file2 alone is different.\n\nYea, your patch looked fine to me, minus the use of subshells where we\ncould avoid it.\n\nI'm fine taking your version.\n\nThanks,\nJake\n"},{"id":"334371","messageId":"20171207211258.GB12850@sigill.intra.peff.net","threadId":"47397","inReplyTo":"20171207190135.28660-1-jacob.e.keller@intel.com","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-07T21:12:58Z","receivedAt":"2017-12-07T21:13:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 07, 2017 at 11:01:35AM -0800, Jacob Keller wrote:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n> \n> We already have tests for --relative, but they currently only test when\n> a prefix has been provided. This fails to test the case where --relative\n> by itself should use the current directory as the prefix.\n> \n> Teach the check_$type functions to take a directory argument to indicate\n> which subdirectory to run the git commands in. Add a new test which uses\n> this to test --relative without a prefix value.\n\nThis looks good to me (and I slightly prefer it over Junio's for the\nsimplicity).\n\nI agree on the ordering suggestion Junio made.\n\n-Peff\n"},{"id":"334372","messageId":"CA+P7+xr-aUm_=50gsrJ3TXpDzvi7LYzWPeamAC+TqzLyh_mY8w@mail.gmail.com","threadId":"47397","inReplyTo":"20171207211258.GB12850@sigill.intra.peff.net","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-12-07T21:20:23Z","receivedAt":"2017-12-07T21:20:59Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 7, 2017 at 1:12 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Dec 07, 2017 at 11:01:35AM -0800, Jacob Keller wrote:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>>\n>> We already have tests for --relative, but they currently only test when\n>> a prefix has been provided. This fails to test the case where --relative\n>> by itself should use the current directory as the prefix.\n>>\n>> Teach the check_$type functions to take a directory argument to indicate\n>> which subdirectory to run the git commands in. Add a new test which uses\n>> this to test --relative without a prefix value.\n>\n> This looks good to me (and I slightly prefer it over Junio's for the\n> simplicity).\n>\n> I agree on the ordering suggestion Junio made.\n>\n> -Peff\n\nAs do I. Junio, if we go with my version, feel free to squash in the\nre-order. Or if you prefer, I can send a v2 (though for such a small\nchange I don't see the benefit).\n\nThanks,\nJake\n"},{"id":"334382","messageId":"xmqqtvx2b41s.fsf@gitster.mtv.corp.google.com","threadId":"47397","inReplyTo":"20171207211258.GB12850@sigill.intra.peff.net","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-07T21:50:55Z","receivedAt":"2017-12-07T21:51:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 07, 2017 at 11:01:35AM -0800, Jacob Keller wrote:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>> \n>> We already have tests for --relative, but they currently only test when\n>> a prefix has been provided. This fails to test the case where --relative\n>> by itself should use the current directory as the prefix.\n>> \n>> Teach the check_$type functions to take a directory argument to indicate\n>> which subdirectory to run the git commands in. Add a new test which uses\n>> this to test --relative without a prefix value.\n>\n> This looks good to me (and I slightly prefer it over Junio's for the\n> simplicity).\n>\n> I agree on the ordering suggestion Junio made.\n\nI also prefer this one over the other one, provided if it is ported\non top of the preliminary clean-up I did in the other one.\n\nThanks.\n\n"},{"id":"334385","messageId":"CA+P7+xoBrStdKtLZ+N3gkQTd1f+hX7D=ACdpk+9ou_NU7QxjFw@mail.gmail.com","threadId":"47397","inReplyTo":"xmqqtvx2b41s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: add tests for --relative without optional prefix value","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-12-07T22:05:33Z","receivedAt":"2017-12-07T22:05:59Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 7, 2017 at 1:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jeff King <peff@peff.net> writes:\n>\n>> On Thu, Dec 07, 2017 at 11:01:35AM -0800, Jacob Keller wrote:\n>>\n>>> From: Jacob Keller <jacob.keller@gmail.com>\n>>>\n>>> We already have tests for --relative, but they currently only test when\n>>> a prefix has been provided. This fails to test the case where --relative\n>>> by itself should use the current directory as the prefix.\n>>>\n>>> Teach the check_$type functions to take a directory argument to indicate\n>>> which subdirectory to run the git commands in. Add a new test which uses\n>>> this to test --relative without a prefix value.\n>>\n>> This looks good to me (and I slightly prefer it over Junio's for the\n>> simplicity).\n>>\n>> I agree on the ordering suggestion Junio made.\n>\n> I also prefer this one over the other one, provided if it is ported\n> on top of the preliminary clean-up I did in the other one.\n>\n> Thanks.\n>\n\nYea. I can do that if you want.\n\nThanks,\nJake\n"}]}