{"thread":{"id":"45868","subject":"[RESEND PATCH] diff: recurse into nested submodules for inline diff","startedAt":"2017-05-04T21:44:05Z","lastAt":"2017-05-08T00:46:53Z","messageCount":3,"participants":["Stefan Beller","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"318802","messageId":"20170504214355.14179-1-sbeller@google.com","threadId":"45868","inReplyTo":null,"subject":"[RESEND PATCH] diff: recurse into nested submodules for inline diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-05-04T21:43:55Z","receivedAt":"2017-05-04T21:44:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When fd47ae6a5b (diff: teach diff to display submodule difference with an\ninline diff, 2016-08-31) was introduced, we did not think of recursing\ninto nested submodules.\n\nWhen showing the inline diff for submodules, automatically recurse\ninto nested submodules as well with inline submodule diffs.\n\nSigned-off-by: Stefan Beller \n---\n\nThis is a resend of \nhttps://public-inbox.org/git/20170331231733.11123-3-sbeller@google.com/\n\nIn the cover letter of that patch, I said I would want to redo the tests with\nscrubbed output. However given the widespread use of unscrubbed output,\nI think this is fine as-is to include.\n\nThanks,\nStefan\n\n submodule.c | 3 +-\n t/t4060-diff-submodule-option-diff-format.sh | 41 ++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex d3299e29c0..2d56a9562e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -554,7 +554,8 @@ void show_submodule_inline_diff(FILE *f, const char *path,\n \tcp.no_stdin = 1;\n \n \t/* TODO: other options may need to be passed here. */\n-\targv_array_push(&cp.args, \"diff\");\n+\targv_array_pushl(&cp.args, \"diff\", \"--submodule=diff\", NULL);\n+\n \targv_array_pushf(&cp.args, \"--line-prefix=%s\", line_prefix);\n \tif (DIFF_OPT_TST(o, REVERSE_DIFF)) {\n \t\targv_array_pushf(&cp.args, \"--src-prefix=%s%s/\",\ndiff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh\nindex d4a3ffa69c..33ec26d755 100755\n--- a/t/t4060-diff-submodule-option-diff-format.sh\n+++ b/t/t4060-diff-submodule-option-diff-format.sh\n@@ -775,4 +775,45 @@ test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'diff --submodule=diff recurses into nested submodules' '\n+\tcat >expected <<-EOF &&\n+\tSubmodule sm2 contains modified content\n+\tSubmodule sm2 a5a65c9..280969a:\n+\tdiff --git a/sm2/.gitmodules b/sm2/.gitmodules\n+\tnew file mode 100644\n+\tindex 0000000..3a816b8\n+\t--- /dev/null\n+\t+++ b/sm2/.gitmodules\n+\t@@ -0,0 +1,3 @@\n+\t+[submodule \"nested\"]\n+\t+\tpath = nested\n+\t+\turl = ../sm2\n+\tSubmodule nested 0000000...b55928c (new submodule)\n+\tdiff --git a/sm2/nested/file b/sm2/nested/file\n+\tnew file mode 100644\n+\tindex 0000000..ca281f5\n+\t--- /dev/null\n+\t+++ b/sm2/nested/file\n+\t@@ -0,0 +1 @@\n+\t+nested content\n+\tdiff --git a/sm2/nested/foo8 b/sm2/nested/foo8\n+\tnew file mode 100644\n+\tindex 0000000..db9916b\n+\t--- /dev/null\n+\t+++ b/sm2/nested/foo8\n+\t@@ -0,0 +1 @@\n+\t+foo8\n+\tdiff --git a/sm2/nested/foo9 b/sm2/nested/foo9\n+\tnew file mode 100644\n+\tindex 0000000..9c3b4f6\n+\t--- /dev/null\n+\t+++ b/sm2/nested/foo9\n+\t@@ -0,0 +1 @@\n+\t+foo9\n+\tEOF\n+\tgit diff --submodule=diff >actual 2>err &&\n+\ttest_must_be_empty err &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.13.0.rc1.18.g9ce9a66034\n\n"},{"id":"318925","messageId":"CA+P7+xqMNiHGi=NVjTHCmnkk3u2BVC5Kfs6p6S3m+LwZr6vkaQ@mail.gmail.com","threadId":"45868","inReplyTo":"20170504214355.14179-1-sbeller@google.com","subject":"Re: [RESEND PATCH] diff: recurse into nested submodules for inline diff","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-05-06T01:24:42Z","receivedAt":"2017-05-06T01:25:09Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, May 4, 2017 at 2:43 PM, Stefan Beller wrote:\n> When fd47ae6a5b (diff: teach diff to display submodule difference with an\n> inline diff, 2016-08-31) was introduced, we did not think of recursing\n> into nested submodules.\n>\n> When showing the inline diff for submodules, automatically recurse\n> into nested submodules as well with inline submodule diffs.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> This is a resend of\n> https://public-inbox.org/git/20170331231733.11123-3-sbeller@google.com/\n>\n> In the cover letter of that patch, I said I would want to redo the tests with\n> scrubbed output. However given the widespread use of unscrubbed output,\n> I think this is fine as-is to include.\n>\n> Thanks,\n> Stefan\n\nFix looks obviously correct to me. I would like to change the tests to\nnot use the unscrubbed output, but that's a problem for a separate\npatch.\n\nThanks,\nJake\n"},{"id":"319030","messageId":"xmqqfuggcgzt.fsf@gitster.mtv.corp.google.com","threadId":"45868","inReplyTo":"CA+P7+xqMNiHGi=NVjTHCmnkk3u2BVC5Kfs6p6S3m+LwZr6vkaQ@mail.gmail.com","subject":"Re: [RESEND PATCH] diff: recurse into nested submodules for inline diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-08T00:46:46Z","receivedAt":"2017-05-08T00:46:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> On Thu, May 4, 2017 at 2:43 PM, Stefan Beller wrote:\n>> When fd47ae6a5b (diff: teach diff to display submodule difference with an\n>> inline diff, 2016-08-31) was introduced, we did not think of recursing\n>> into nested submodules.\n>>\n>> When showing the inline diff for submodules, automatically recurse\n>> into nested submodules as well with inline submodule diffs.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> This is a resend of\n>> https://public-inbox.org/git/20170331231733.11123-3-sbeller@google.com/\n>>\n>> In the cover letter of that patch, I said I would want to redo the tests with\n>> scrubbed output. However given the widespread use of unscrubbed output,\n>> I think this is fine as-is to include.\n>>\n>> Thanks,\n>> Stefan\n>\n> Fix looks obviously correct to me. I would like to change the tests to\n> not use the unscrubbed output, but that's a problem for a separate\n> patch.\n\nOK.\n\nThanks, both.\n"}]}