{"thread":{"id":"52288","subject":"[PATCH 0/3] range-diff: don't compare notes","startedAt":"2019-11-19T01:06:57Z","lastAt":"2019-11-21T18:35:33Z","messageCount":32,"participants":["Denton Liu","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"386489","messageId":"cover.1574125554.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":null,"subject":"[PATCH 0/3] range-diff: don't compare notes","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T01:06:52Z","receivedAt":"2019-11-19T01:06:57Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When I was using range-diff at $DAYJOB earlier, I realised that it\nincludes commit notes as part of the commit message comparison. This is\nundesired behaviour so this patchset documents it and stops it from\nhappening.\n\nDenton Liu (3):\n t3206: remove spaces after redirect operators\n t3206: demonstrate failure with notes\n range-diff: use --no-notes to generate patches\n\n range-diff.c | 2 +-\n t/t3206-range-diff.sh | 17 ++++++++++++++++-\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386490","messageId":"6966a30a5a510263f11758eaab53f2d58b68efe5.1574125554.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574125554.git.liu.denton@gmail.com","subject":"[PATCH 1/3] t3206: remove spaces after redirect operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T01:06:55Z","receivedAt":"2019-11-19T01:06:59Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For shell scripts, the usual convention is for there to be no space\nafter redirection operators, (e.g. `>file`, not `> file`). Remove the\none instance of this.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 0579cd9969..87c6c029db 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -8,7 +8,7 @@ test_description='range-diff tests'\n # harm than good. We need some real history.\n \n test_expect_success 'setup' '\n-\tgit fast-import < \"$TEST_DIRECTORY\"/t3206/history.export &&\n+\tgit fast-import <\"$TEST_DIRECTORY\"/t3206/history.export &&\n \ttest_oid_cache <<-EOF\n \t# topic\n \tt1 sha1:4de457d\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386491","messageId":"00d6b47db0a68d0bc91b252675a2165985426f5e.1574125554.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574125554.git.liu.denton@gmail.com","subject":"[PATCH 2/3] t3206: demonstrate failure with notes","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T01:06:57Z","receivedAt":"2019-11-19T01:07:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When a commit being range-diff'd has a note attached to it, the note\nwill be compared as well. However, this shouldn't happen since the\npurpose of range-diff is to compare the difference between two\ncommit-ranges and, since the note attached to a commit is mutable, they\nshouldn't be included as part of the comparison.\n\nDemonstrate this failure in t3206.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 87c6c029db..29b9a6f854 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -505,4 +505,19 @@ test_expect_success 'range-diff overrides diff.noprefix internally' '\n \tgit -c diff.noprefix=true range-diff HEAD^...\n '\n \n+test_expect_failure 'range-diff does not compare notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit range-diff --no-color master..topic master..unmodified \\\n+\t\t>actual &&\n+\tcat >expected <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386492","messageId":"48e850df876eccd3c6cda49ee048565b2592e33b.1574125554.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574125554.git.liu.denton@gmail.com","subject":"[PATCH 3/3] range-diff: use --no-notes to generate patches","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T01:06:59Z","receivedAt":"2019-11-19T01:07:04Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When a commit being range-diff'd has a note attached to it, the note\nwill be compared as well. However, this shouldn't happen since the\npurpose of range-diff is to compare the difference between two\ncommit-ranges and, since the note attached to a commit is mutable, they\nshouldn't be included as part of the comparison.\n\nAdd `--no-notes` to the `git log` call and mark the corresponding test\nas passing.\n\nSigned-off-by: Denton Liu \n---\n range-diff.c | 2 +-\n t/t3206-range-diff.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 7fed5a3b4b..725930ee92 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -52,7 +52,7 @@ static int read_patches(const char *range, struct string_list *list)\n \n \targv_array_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t\t\"--reverse\", \"--date-order\", \"--decorate=no\",\n-\t\t\t\"--no-prefix\",\n+\t\t\t\"--no-prefix\", \"--no-notes\",\n \t\t\t/*\n \t\t\t * Choose indicators that are not used anywhere\n \t\t\t * else in diffs, but still look reasonable\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 29b9a6f854..671703f85b 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -505,7 +505,7 @@ test_expect_success 'range-diff overrides diff.noprefix internally' '\n \tgit -c diff.noprefix=true range-diff HEAD^...\n '\n \n-test_expect_failure 'range-diff does not compare notes' '\n+test_expect_success 'range-diff does not compare notes' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n \ttest_when_finished git notes remove topic unmodified &&\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386500","messageId":"xmqqftikzi8f.fsf@gitster-ct.c.googlers.com","threadId":"52288","inReplyTo":"cover.1574125554.git.liu.denton@gmail.com","subject":"Re: [PATCH 0/3] range-diff: don't compare notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T02:56:48Z","receivedAt":"2019-11-19T02:56:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> When I was using range-diff at $DAYJOB earlier, I realised that it\n> includes commit notes as part of the commit message comparison. This is\n> undesired behaviour so this patchset documents it and stops it from\n> happening.\n\nI actually wish it allowed me to compare them _with_ --notes=\nspecified by the user. If you are passing --no-notes through,\nperhaps you can also pass through such an option while at it.\n"},{"id":"386501","messageId":"xmqqblt8zhxh.fsf@gitster-ct.c.googlers.com","threadId":"52288","inReplyTo":"00d6b47db0a68d0bc91b252675a2165985426f5e.1574125554.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/3] t3206: demonstrate failure with notes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T03:03:22Z","receivedAt":"2019-11-19T03:03:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> When a commit being range-diff'd has a note attached to it, the note\n> will be compared as well. However, this shouldn't happen since the\n> purpose of range-diff is to compare the difference between two\n> commit-ranges and, since the note attached to a commit is mutable, they\n> shouldn't be included as part of the comparison.\n\nI do not agree with the reasoning. Commits in the new iteration of\nthe same series would have note attached to them that are different\nfrom the note attached to corresponding commits in the old iteration.\n\nImagine that \"git am\" gets enhanced to store the per-iteration\ncomments you write under the three-dash lines (e.g. \"fixed typo in\nthe log message pointed out by X\") as notes attached to each of the\nresulting commits. It does make sense to compare the commits _with_\nnotes by default, if these notes are configured to be shown in \"git\nshow\" output by default for the user.\n\nI do think that it may be a good idea to optionally allow comparison\nwithout notes, when \"--no-notes\" is given to range-diff on the\ncommand line, by passing the option through, though.\n\nThanks.\n\n"},{"id":"386569","messageId":"cover.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574125554.git.liu.denton@gmail.com","subject":"[PATCH v2 0/8] range-diff: learn `--notes`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:27Z","receivedAt":"2019-11-19T23:55:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This patchset teaches range-diff and format-patch to pass `--notes`\noptions down to the `git log` machinery. This should make the behaviour\nmore configurable for users who either don't want notes to be displayed\nor want multiple notes refs to be displayed.\n\nChanges since v1:\n\n* Complete overhaul of approach\n\nDenton Liu (8):\n argv-array: add space after `while`\n rev-list-options.txt: remove reference to --show-notes\n t3206: remove spaces after redirect operators\n t3206: s/expected/expect/\n t3206: demonstrate current notes behavior\n range-diff: output `## Notes ##` header\n range-diff: passthrough --[no-]notes to `git log`\n format-patch: pass notes configuration to range-diff\n\n Documentation/git-range-diff.txt | 6 +-\n Documentation/rev-list-options.txt | 2 +-\n argv-array.c | 2 +-\n builtin/log.c | 24 ++-\n builtin/range-diff.c | 6 +-\n log-tree.c | 2 +-\n range-diff.c | 21 ++-\n range-diff.h | 4 +-\n t/t3206-range-diff.sh | 264 +++++++++++++++++++++++++----\n 9 files changed, 286 insertions(+), 45 deletions(-)\n\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386570","messageId":"fd78742570bbeaa4096eb0cce24fe33054aed176.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 1/8] argv-array: add space after `while`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:29Z","receivedAt":"2019-11-19T23:55:39Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n argv-array.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/argv-array.c b/argv-array.c\nindex f352ea9357..61ef8c0dfd 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -46,7 +46,7 @@ void argv_array_pushl(struct argv_array *array, ...)\n \tconst char *arg;\n \n \tva_start(ap, array);\n-\twhile((arg = va_arg(ap, const char *)))\n+\twhile ((arg = va_arg(ap, const char *)))\n \t\targv_array_push(array, arg);\n \tva_end(ap);\n }\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386571","messageId":"e1b023a6fc2818f871e3033bd21f72a2de7b7a21.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 2/8] rev-list-options.txt: remove reference to --show-notes","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:32Z","receivedAt":"2019-11-19T23:55:39Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In ab18b2c0df (\"log/pretty-options: Document --[no-]notes and deprecate\nold notes options\", 2011-03-30), the `--show-notes` option was\ndeprecated. However, this reference to it still remains. Change it to\nreference the replacement option: `--notes`.\n\nSigned-off-by: Denton Liu \n---\n Documentation/rev-list-options.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 90ff9e2bea..311bc06a9b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -58,7 +58,7 @@ endif::git-rev-list[]\n \t`--all-match`).\n ifndef::git-rev-list[]\n +\n-When `--show-notes` is in effect, the message from the notes is\n+When `--notes` is in effect, the message from the notes is\n matched as if it were part of the log message.\n endif::git-rev-list[]\n \n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386572","messageId":"16a90b70638b100afe4b7d68c0db20ffae7ce964.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 3/8] t3206: remove spaces after redirect operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:34Z","receivedAt":"2019-11-19T23:55:39Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For shell scripts, the usual convention is for there to be no space\nafter redirection operators, (e.g. `>file`, not `> file`). Remove the\none instance of this.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 0579cd9969..87c6c029db 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -8,7 +8,7 @@ test_description='range-diff tests'\n # harm than good. We need some real history.\n \n test_expect_success 'setup' '\n-\tgit fast-import < \"$TEST_DIRECTORY\"/t3206/history.export &&\n+\tgit fast-import <\"$TEST_DIRECTORY\"/t3206/history.export &&\n \ttest_oid_cache <<-EOF\n \t# topic\n \tt1 sha1:4de457d\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386573","messageId":"2bbcafa08ae201f9612d37f6c7962a221e5d1d8a.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 4/8] t3206: s/expected/expect/","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:37Z","receivedAt":"2019-11-19T23:55:42Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For test cases, the usual convention is to name expected output files\n\"expect\", not \"expected\". Replace all instances of \"expected\" with\n\"expect\".\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 64 +++++++++++++++++++++----------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 87c6c029db..e14b4951bb 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -121,88 +121,88 @@ test_expect_success 'setup' '\n test_expect_success 'simple A..B A..C (unmodified)' '\n \tgit range-diff --no-color master..topic master..unmodified \\\n \t\t>actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'simple B...C (unmodified)' '\n \tgit range-diff --no-color topic...unmodified >actual &&\n-\t# same \"expected\" as above\n-\ttest_cmp expected actual\n+\t# same \"expect\" as above\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'simple A B C (unmodified)' '\n \tgit range-diff --no-color master topic unmodified >actual &&\n-\t# same \"expected\" as above\n-\ttest_cmp expected actual\n+\t# same \"expect\" as above\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid r1) s/5/A/\n \t3: $(test_oid t3) = 2: $(test_oid r2) s/11/B/\n \t4: $(test_oid t4) = 3: $(test_oid r3) s/12/B/\n \t2: $(test_oid t2) = 4: $(test_oid r4) s/4/A/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'removed a commit' '\n \tgit range-diff --no-color master topic removed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid d1) s/5/A/\n \t2: $(test_oid t2) < -: $(test_oid __) s/4/A/\n \t3: $(test_oid t3) = 2: $(test_oid d2) s/11/B/\n \t4: $(test_oid t4) = 3: $(test_oid d3) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'added a commit' '\n \tgit range-diff --no-color master topic added >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid a1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid a2) s/4/A/\n \t-: $(test_oid __) > 3: $(test_oid a3) s/6/A/\n \t3: $(test_oid t3) = 4: $(test_oid a4) s/11/B/\n \t4: $(test_oid t4) = 5: $(test_oid a5) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'new base, A B C' '\n \tgit range-diff --no-color master topic rebased >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid b1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid b2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid b3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid b4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'new base, B...C' '\n \t# this syntax includes the commits from master!\n \tgit range-diff --no-color topic...rebased >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t-: $(test_oid __) > 1: $(test_oid b5) unrelated\n \t1: $(test_oid t1) = 2: $(test_oid b1) s/5/A/\n \t2: $(test_oid t2) = 3: $(test_oid b2) s/4/A/\n \t3: $(test_oid t3) = 4: $(test_oid b3) s/11/B/\n \t4: $(test_oid t4) = 5: $(test_oid b4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit' '\n \tgit range-diff --no-color topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n@@ -226,23 +226,23 @@ test_expect_success 'changed commit' '\n \t +B\n \t 13\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with --no-patch diff option' '\n \tgit range-diff --no-color --no-patch topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n \t4: $(test_oid t4) ! 4: $(test_oid c4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with --stat diff option' '\n \tgit range-diff --no-color --stat topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t a => b | 0\n \t 1 file changed, 0 insertions(+), 0 deletions(-)\n@@ -256,12 +256,12 @@ test_expect_success 'changed commit with --stat diff option' '\n \t a => b | 0\n \t 1 file changed, 0 insertions(+), 0 deletions(-)\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with sm config' '\n \tgit range-diff --no-color --submodule=log topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n@@ -285,12 +285,12 @@ test_expect_success 'changed commit with sm config' '\n \t +B\n \t 13\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'renamed file' '\n \tgit range-diff --no-color --submodule=log topic...renamed-file >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid n1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid n2) s/4/A/\n \t @@ Metadata\n@@ -330,12 +330,12 @@ test_expect_success 'renamed file' '\n \t Z 10\n \t Z B\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'file with mode only change' '\n \tgit range-diff --no-color --submodule=log topic...mode-only-change >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: fccce22 ! 1: 4d39cb3 s/4/A/\n \t @@ Metadata\n \t ZAuthor: Thomas Rast \n@@ -370,12 +370,12 @@ test_expect_success 'file with mode only change' '\n \t + ## other-file (mode change 100644 => 100755) ##\n \t3: a63e992 = 3: 4c1e0f5 s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'file added and later removed' '\n \tgit range-diff --no-color --submodule=log topic...added-removed >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid s1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid s2) s/4/A/\n \t @@ Metadata\n@@ -411,7 +411,7 @@ test_expect_success 'file added and later removed' '\n \t + ## new-file (deleted) ##\n \t4: $(test_oid t4) = 4: $(test_oid s4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'no commits on one side' '\n@@ -421,7 +421,7 @@ test_expect_success 'no commits on one side' '\n \n test_expect_success 'changed message' '\n \tgit range-diff --no-color topic...changed-message >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid m1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid m2) s/4/A/\n \t @@ Metadata\n@@ -436,7 +436,7 @@ test_expect_success 'changed message' '\n \t3: $(test_oid t3) = 3: $(test_oid m3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid m4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'dual-coloring' '\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386574","messageId":"4cf7d39fe342a1f140598f09369afaf785358aa5.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 5/8] t3206: demonstrate current notes behavior","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:39Z","receivedAt":"2019-11-19T23:55:45Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test suite had a blindspot where it did not check the behavior of\nrange-diff and format-patch when notes were present. Cover this\nblindspot.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 52 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 52 insertions(+)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex e14b4951bb..69babdcf03 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -505,4 +505,56 @@ test_expect_success 'range-diff overrides diff.noprefix internally' '\n \tgit -c diff.noprefix=true range-diff HEAD^...\n '\n \n+test_expect_success 'range-diff compares notes by default' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit range-diff --no-color master..topic master..unmodified \\\n+\t\t>actual &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n+\t @@ Metadata\n+\t Z\n+\t Z ## Commit message ##\n+\t Z s/12/B/\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'format-patch --range-diff compares notes by default' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Metadata\n+\t Z\n+\t Z ## Commit message ##\n+\t Z s/12/B/\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Metadata/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386575","messageId":"abba44082930450bbe577585f6c294c5f796a8ef.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 6/8] range-diff: output `## Notes ##` header","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:42Z","receivedAt":"2019-11-19T23:55:47Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When notes were included in the output of range-diff, they were just\nmashed together with the rest of the commit message. As a result, users\nwouldn't be able to clearly distinguish where the commit message ended\nand where the notes started.\n\nOutput a `## Notes ##` header when notes are detected so that notes can\nbe compared more clearly.\n\nNote that we handle case of `Notes (): -> ## Notes () ##` with\nthis code as well. We can't test this in this patch, however, since\nthere is currently no way to pass along different notes refs to `git\nlog`. This will be fixed in a future patch.\n\nSigned-off-by: Denton Liu \n---\n range-diff.c | 6 ++++++\n t/t3206-range-diff.sh | 14 +++++++-------\n 2 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 7fed5a3b4b..623397221d 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -144,6 +144,12 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t\tstrbuf_addstr(&buf, line);\n \t\t\t\tstrbuf_addstr(&buf, \"\\n\\n\");\n \t\t\t\tstrbuf_addstr(&buf, \" ## Commit message ##\\n\");\n+\t\t\t} else if (starts_with(line, \"Notes\") &&\n+\t\t\t\t line[strlen(line) - 1] == ':') {\n+\t\t\t\tstrbuf_addstr(&buf, \"\\n\\n\");\n+\t\t\t\t/* strip the trailing colon */\n+\t\t\t\tstrbuf_addf(&buf, \" ## %.*s ##\\n\",\n+\t\t\t\t\t (int)(strlen(line) - 1), line);\n \t\t\t} else if (starts_with(line, \" \")) {\n \t\t\t\tp = line + len - 2;\n \t\t\t\twhile (isspace(*p) && p >= line)\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 69babdcf03..5bb33efdff 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -516,10 +516,10 @@ test_expect_success 'range-diff compares notes by default' '\n \t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n \t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n-\t @@ Metadata\n+\t @@ Commit message\n \t Z\n-\t Z ## Commit message ##\n-\t Z s/12/B/\n+\t Z\n+\t Z ## Notes ##\n \t - topic note\n \t + unmodified note\n \t Z\n@@ -543,17 +543,17 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgrep \"= 3: .* s/11/B\" 0000-* &&\n \tgrep \"! 4: .* s/12/B\" 0000-* &&\n \tsed s/Z/\\ /g >expect <<-EOF &&\n-\t @@ Metadata\n+\t @@ Commit message\n \t Z\n-\t Z ## Commit message ##\n-\t Z s/12/B/\n+\t Z\n+\t Z ## Notes ##\n \t - topic note\n \t + unmodified note\n \t Z\n \t Z ## file ##\n \t Z@@ file: A\n \tEOF\n-\tsed \"/@@ Metadata/,/@@ file: A/!d\" 0000-* >actual &&\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386576","messageId":"0cb86b383b9c115c9c6077d47f0c124a96b30acf.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:44Z","receivedAt":"2019-11-19T23:55:49Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When a commit being range-diff'd has a note attached to it, the note\nwill be compared as well. However, if a user has multiple notes refs or\nif they want to suppress notes from being printed, there is currently no\nway to do this.\n\nPassthrough `---no--notes` to the `git log` call so that this option is\ncustomizable.\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-range-diff.txt | 6 +++-\n builtin/log.c | 2 +-\n builtin/range-diff.c | 6 +++-\n log-tree.c | 2 +-\n range-diff.c | 15 ++++++----\n range-diff.h | 4 ++-\n t/t3206-range-diff.sh | 47 ++++++++++++++++++++++++++++++++\n 7 files changed, 72 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 8a6ea2c6c5..a3d69713f7 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -57,6 +57,10 @@ to revert to color all lines according to the outer diff markers\n \tSee the ``Algorithm`` section below for an explanation why this is\n \tneeded.\n \n+--[no-]notes[=]::\n+\tThis flag is passed to the `git log` program\n+\t(see linkgit:git-log[1]) that generates the patches.\n+\n ::\n \tCompare the commits specified by the two ranges, where\n \t`` is considered an older version of ``.\n@@ -75,7 +79,7 @@ to revert to color all lines according to the outer diff markers\n linkgit:git-diff[1]), most notably the `--color=[]` and\n `--no-color` options. These options are used when generating the \"diff\n between patches\", i.e. to compare the author, commit message and diff of\n-corresponding old/new commits. There is currently no means to tweak the\n+corresponding old/new commits. There is currently no means to tweak most of the\n diff options passed to `git log` when generating those patches.\n \n OUTPUT STABILITY\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a26f223ab4..047ac4594d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1189,7 +1189,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n \t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts);\n+\t\t\t\trev->creation_factor, 1, &opts, NULL);\n \t}\n }\n \ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 9202e75544..98acf3533e 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -15,12 +15,16 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n \tstruct diff_options diffopt = { NULL };\n+\tstruct argv_array other_arg = ARGV_ARRAY_INIT;\n \tint simple_color = -1;\n \tstruct option range_diff_options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n \t\t\t N_(\"use simple diff colors\")),\n+\t\tOPT_PASSTHRU_ARGV(0, \"notes\", &other_arg,\n+\t\t\t\t N_(\"notes\"), N_(\"passed to 'git log'\"),\n+\t\t\t\t PARSE_OPT_OPTARG),\n \t\tOPT_END()\n \t};\n \tstruct option *options;\n@@ -78,7 +82,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \tFREE_AND_NULL(options);\n \n \tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n-\t\t\t simple_color < 1, &diffopt);\n+\t\t\t simple_color < 1, &diffopt, &other_arg);\n \n \tstrbuf_release(&range1);\n \tstrbuf_release(&range2);\ndiff --git a/log-tree.c b/log-tree.c\nindex 923a299e70..151e12f415 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -770,7 +770,7 @@ void show_log(struct rev_info *opt)\n \t\topts.use_color = opt->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n-\t\t\t\topt->creation_factor, 1, &opts);\n+\t\t\t\topt->creation_factor, 1, &opts, NULL);\n \n \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n \t}\ndiff --git a/range-diff.c b/range-diff.c\nindex 623397221d..f56b4012a2 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -40,7 +40,8 @@ static size_t find_end_of_line(char *buffer, unsigned long size)\n * Reads the patches into a string list, with the `util` field being populated\n * as struct object_id (will need to be free()d).\n */\n-static int read_patches(const char *range, struct string_list *list)\n+static int read_patches(const char *range, struct string_list *list,\n+\t\t\tstruct argv_array *other_arg)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n@@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t\"--output-indicator-new=>\",\n \t\t\t\"--output-indicator-old=<\",\n \t\t\t\"--output-indicator-context=#\",\n-\t\t\t\"--no-abbrev-commit\", range,\n+\t\t\t\"--no-abbrev-commit\",\n \t\t\tNULL);\n+\tif (other_arg)\n+\t\targv_array_pushv(&cp.args, other_arg->argv);\n+\targv_array_push(&cp.args, range);\n \tcp.out = -1;\n \tcp.no_stdin = 1;\n \tcp.git_cmd = 1;\n@@ -502,16 +506,17 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n \n int show_range_diff(const char *range1, const char *range2,\n \t\t int creation_factor, int dual_color,\n-\t\t struct diff_options *diffopt)\n+\t\t struct diff_options *diffopt,\n+\t\t struct argv_array *other_arg)\n {\n \tint res = 0;\n \n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1))\n+\tif (read_patches(range1, &branch1, other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n-\tif (!res && read_patches(range2, &branch2))\n+\tif (!res && read_patches(range2, &branch2, other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range2);\n \n \tif (!res) {\ndiff --git a/range-diff.h b/range-diff.h\nindex 08a50b6e98..7d918ab9ed 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -2,6 +2,7 @@\n #define RANGE_DIFF_H\n \n #include \"diff.h\"\n+#include \"argv-array.h\"\n \n #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n \n@@ -12,6 +13,7 @@\n */\n int show_range_diff(const char *range1, const char *range2,\n \t\t int creation_factor, int dual_color,\n-\t\t struct diff_options *diffopt);\n+\t\t struct diff_options *diffopt,\n+\t\t struct argv_array *other_arg);\n \n #endif\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 5bb33efdff..cea1846282 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -529,6 +529,53 @@ test_expect_success 'range-diff compares notes by default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'range-diff with --no-notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit range-diff --no-color --no-notes master..topic master..unmodified \\\n+\t\t>actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'range-diff with multiple --notes' '\n+\tgit notes --ref=note1 add -m \"topic note1\" topic &&\n+\tgit notes --ref=note1 add -m \"unmodified note1\" unmodified &&\n+\ttest_when_finished git notes --ref=note1 remove topic unmodified &&\n+\tgit notes --ref=note2 add -m \"topic note2\" topic &&\n+\tgit notes --ref=note2 add -m \"unmodified note2\" unmodified &&\n+\ttest_when_finished git notes --ref=note2 remove topic unmodified &&\n+\tgit range-diff --no-color --notes=note1 --notes=note2 master..topic master..unmodified \\\n+\t\t>actual &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes (note1) ##\n+\t - topic note1\n+\t + unmodified note1\n+\t Z\n+\t Z\n+\t Z ## Notes (note2) ##\n+\t - topic note2\n+\t + unmodified note2\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386577","messageId":"0b7fd66b4dc91947542b56441782766f8ad81690.1574207673.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v2 8/8] format-patch: pass notes configuration to range-diff","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-19T23:55:47Z","receivedAt":"2019-11-19T23:55:51Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Since format-patch accepts `--[no-]notes`, one would expect the\nrange-diff generated to also respect the setting. Unfortunately, the\nrange-diff we currently generate only uses the default option (which\nalways outputs default notes, even when notes are not being used\nelsewhere).\n\nPass the notes configuration to range-diff so that it can honor it.\n\nSigned-off-by: Denton Liu \n---\n builtin/log.c | 24 +++++++++-\n t/t3206-range-diff.sh | 101 +++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 123 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 047ac4594d..e192f219d9 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1111,6 +1111,25 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n \tstrbuf_release(&subject_sb);\n }\n \n+static int get_notes_refs(struct string_list_item *item, void *arg)\n+{\n+\targv_array_pushf(arg, \"--notes=%s\", item->string);\n+\treturn 0;\n+}\n+\n+static void get_notes_args(struct argv_array *arg, struct rev_info *rev)\n+{\n+\tif (!rev->show_notes) {\n+\t\targv_array_push(arg, \"--no-notes\");\n+\t} else if (rev->notes_opt.use_default_notes > 0 ||\n+\t\t (rev->notes_opt.use_default_notes == -1 &&\n+\t\t !rev->notes_opt.extra_notes_refs.nr)) {\n+\t\targv_array_push(arg, \"--notes\");\n+\t} else {\n+\t\tfor_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg);\n+\t}\n+}\n+\n static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n@@ -1183,13 +1202,16 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t * can be added later if deemed desirable.\n \t\t */\n \t\tstruct diff_options opts;\n+\t\tstruct argv_array other_arg = ARGV_ARRAY_INIT;\n \t\tdiff_setup(&opts);\n \t\topts.file = rev->diffopt.file;\n \t\topts.use_color = rev->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n+\t\tget_notes_args(&other_arg, rev);\n \t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts, NULL);\n+\t\t\t\trev->creation_factor, 1, &opts, &other_arg);\n+\t\targv_array_clear(&other_arg);\n \t}\n }\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex cea1846282..abbf1efc20 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -576,7 +576,7 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'format-patch --range-diff compares notes by default' '\n+test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n \ttest_when_finished git notes remove topic unmodified &&\n@@ -588,6 +588,40 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgrep \"= 1: .* s/5/A\" 0000-* &&\n \tgrep \"= 2: .* s/4/A\" 0000-* &&\n \tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"= 4: .* s/12/B\" 0000-* &&\n+\t! grep \"Notes\" 0000-* &&\n+\t! grep \"note\" 0000-*\n+'\n+\n+test_expect_success 'format-patch --range-diff with --no-notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --no-notes --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"= 4: .* s/12/B\" 0000-* &&\n+\t! grep \"Notes\" 0000-* &&\n+\t! grep \"note\" 0000-*\n+'\n+\n+test_expect_success 'format-patch --range-diff with --notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --notes --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n \tgrep \"! 4: .* s/12/B\" 0000-* &&\n \tsed s/Z/\\ /g >expect <<-EOF &&\n \t @@ Commit message\n@@ -604,4 +638,69 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch --range-diff with --notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_config format.notes true &&\n+\tgit format-patch --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes ##\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'format-patch --range-diff with multiple notes' '\n+\tgit notes --ref=note1 add -m \"topic note1\" topic &&\n+\tgit notes --ref=note1 add -m \"unmodified note1\" unmodified &&\n+\ttest_when_finished git notes --ref=note1 remove topic unmodified &&\n+\tgit notes --ref=note2 add -m \"topic note2\" topic &&\n+\tgit notes --ref=note2 add -m \"unmodified note2\" unmodified &&\n+\ttest_when_finished git notes --ref=note2 remove topic unmodified &&\n+\tgit format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes (note1) ##\n+\t - topic note1\n+\t + unmodified note1\n+\t Z\n+\t Z\n+\t Z ## Notes (note2) ##\n+\t - topic note2\n+\t + unmodified note2\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.24.0.420.g9ac4901264\n\n"},{"id":"386578","messageId":"CAPig+cRnbnj3-ZjXiRkHU8JNREo1qCvbV9AYcq0TdUcQ8i4VhA@mail.gmail.com","threadId":"52288","inReplyTo":"16a90b70638b100afe4b7d68c0db20ffae7ce964.1574207673.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 3/8] t3206: remove spaces after redirect operators","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-11-20T00:20:26Z","receivedAt":"2019-11-20T00:20:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 19, 2019 at 6:55 PM Denton Liu wrote:\n> For shell scripts, the usual convention is for there to be no space\n> after redirection operators, (e.g. `>file`, not `> file`). Remove the\n> one instance of this.\n>\n> Signed-off-by: Denton Liu \n> ---\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> @@ -8,7 +8,7 @@ test_description='range-diff tests'\n> test_expect_success 'setup' '\n> - git fast-import < \"$TEST_DIRECTORY\"/t3206/history.export &&\n> + git fast-import <\"$TEST_DIRECTORY\"/t3206/history.export &&\n> test_oid_cache <<-EOF\n> # topic\n> t1 sha1:4de457d\n\nWhile you're here cleaning style problems, you could also change this\nhere-doc from -EOF to -\\EOF. (This is the only here-doc in this script\nwhich can use -\\EOF; all the others need -EOF.)\n"},{"id":"386611","messageId":"xmqq1ru3chaw.fsf@gitster-ct.c.googlers.com","threadId":"52288","inReplyTo":"4cf7d39fe342a1f140598f09369afaf785358aa5.1574207673.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 5/8] t3206: demonstrate current notes behavior","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-20T04:17:43Z","receivedAt":"2019-11-20T04:17:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> Subject: Re: [PATCH v2 5/8] t3206: demonstrate current notes behavior\n\nThis makes it sound as if \"current\" is either bad or stale and will\nbe \"fixed\". In the previous round that might have been your\nintention, but I do not think the focus of v2 is that, so phrasing\nmay want to be updated, perhaps like\n\n t3206: range-diff compares log message with commit notes\n\nor something.\n\n> The test suite had a blindspot where it did not check the behavior of\n> range-diff and format-patch when notes were present. Cover this\n> blindspot.\n\nGood description.\n"},{"id":"386612","messageId":"xmqqwobvb2cj.fsf@gitster-ct.c.googlers.com","threadId":"52288","inReplyTo":"0cb86b383b9c115c9c6077d47f0c124a96b30acf.1574207673.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-20T04:26:04Z","receivedAt":"2019-11-20T04:26:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> When a commit being range-diff'd has a note attached to it, the note\n> will be compared as well. However, if a user has multiple notes refs or\n> if they want to suppress notes from being printed, there is currently no\n> way to do this.\n>\n> Passthrough `---no--notes` to the `git log` call so that this option is\n\n\"`--[no-]notes`\" or perhaps \"`--[no-]notes` and `--notes=`\"?\n\nI think the verb phrase is two words, \"pass through\", by the way.\n\n> +--[no-]notes[=]::\n> +\tThis flag is passed to the `git log` program\n> +\t(see linkgit:git-log[1]) that generates the patches.\n\nI can see this was taken from \"git log --help\", and it probably\nneeds fixing for consistency as well, but I think --notes=\nwould be easier to click users' minds with notes.displayRef\nconfiguration variable.\n\n> @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list)\n> \t\t\t\"--output-indicator-new=>\",\n> \t\t\t\"--output-indicator-old=<\",\n> \t\t\t\"--output-indicator-context=#\",\n> -\t\t\t\"--no-abbrev-commit\", range,\n> +\t\t\t\"--no-abbrev-commit\",\n> \t\t\tNULL);\n> +\tif (other_arg)\n> +\t\targv_array_pushv(&cp.args, other_arg->argv);\n> +\targv_array_push(&cp.args, range);\n\nMakes sense.\n\n> diff --git a/range-diff.h b/range-diff.h\n> index 08a50b6e98..7d918ab9ed 100644\n> --- a/range-diff.h\n> +++ b/range-diff.h\n> @@ -2,6 +2,7 @@\n> #define RANGE_DIFF_H\n> \n> #include \"diff.h\"\n> +#include \"argv-array.h\"\n> \n> #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n> \n> @@ -12,6 +13,7 @@\n> */\n> int show_range_diff(const char *range1, const char *range2,\n> \t\t int creation_factor, int dual_color,\n> -\t\t struct diff_options *diffopt);\n> +\t\t struct diff_options *diffopt,\n> +\t\t struct argv_array *other_arg);\n> \n> #endif\n\nI thought a mere use of \"pointer to a struct\" did not have to bring\nthe definition of the struct into the picture? In other words,\nwouldn't it be fine to leave the other_arg a pointer to an opaque\nstructure by not including \"argv-array.h\" in this file?\n\nThanks.\n"},{"id":"386651","messageId":"20191120191258.GA73969@generichostname","threadId":"52288","inReplyTo":"xmqqwobvb2cj.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T19:12:58Z","receivedAt":"2019-11-20T19:13:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Junio,\n\nOn Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote:\n> Denton Liu writes:\n> \n> > When a commit being range-diff'd has a note attached to it, the note\n> > will be compared as well. However, if a user has multiple notes refs or\n> > if they want to suppress notes from being printed, there is currently no\n> > way to do this.\n> >\n> > Passthrough `---no--notes` to the `git log` call so that this option is\n> \n> \"`--[no-]notes`\" or perhaps \"`--[no-]notes` and `--notes=`\"?\n\nWhoops, I probably typed `ys-` in vim instead of `ys[` so I surrounded\nthe `no-` with the wrong characters.\n\n> \n> I think the verb phrase is two words, \"pass through\", by the way.\n\nThanks, I didn't know this.\n\n> \n> > +--[no-]notes[=]::\n> > +\tThis flag is passed to the `git log` program\n> > +\t(see linkgit:git-log[1]) that generates the patches.\n> \n> I can see this was taken from \"git log --help\", and it probably\n> needs fixing for consistency as well, but I think --notes=\n> would be easier to click users' minds with notes.displayRef\n> configuration variable.\n\nI'll put in a cleanup patch for this as well.\n\n> \n> > @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list)\n> > \t\t\t\"--output-indicator-new=>\",\n> > \t\t\t\"--output-indicator-old=<\",\n> > \t\t\t\"--output-indicator-context=#\",\n> > -\t\t\t\"--no-abbrev-commit\", range,\n> > +\t\t\t\"--no-abbrev-commit\",\n> > \t\t\tNULL);\n> > +\tif (other_arg)\n> > +\t\targv_array_pushv(&cp.args, other_arg->argv);\n> > +\targv_array_push(&cp.args, range);\n> \n> Makes sense.\n> \n> > diff --git a/range-diff.h b/range-diff.h\n> > index 08a50b6e98..7d918ab9ed 100644\n> > --- a/range-diff.h\n> > +++ b/range-diff.h\n> > @@ -2,6 +2,7 @@\n> > #define RANGE_DIFF_H\n> > \n> > #include \"diff.h\"\n> > +#include \"argv-array.h\"\n> > \n> > #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n> > \n> > @@ -12,6 +13,7 @@\n> > */\n> > int show_range_diff(const char *range1, const char *range2,\n> > \t\t int creation_factor, int dual_color,\n> > -\t\t struct diff_options *diffopt);\n> > +\t\t struct diff_options *diffopt,\n> > +\t\t struct argv_array *other_arg);\n> > \n> > #endif\n> \n> I thought a mere use of \"pointer to a struct\" did not have to bring\n> the definition of the struct into the picture? In other words,\n> wouldn't it be fine to leave the other_arg a pointer to an opaque\n> structure by not including \"argv-array.h\" in this file?\n\nWithout including \"argv-array.h\", we get the following hdr-check error:\n\n\t$ make range-diff.hco\n\t HDR range-diff.h\n\tIn file included from range-diff.hcc:2:\n\t./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility]\n\t\t\t struct argv_array *other_arg);\n\t\t\t\t ^\n\t1 error generated.\n\tmake: *** [range-diff.hco] Error 1\n\nI am currently using this compiler for reference:\n\n\t$ clang --version\n\tApple LLVM version 10.0.1 (clang-1001.0.46.4)\n\tTarget: x86_64-apple-darwin18.7.0\n\tThread model: posix\n\tInstalledDir: /Library/Developer/CommandLineTools/usr/bin\n\nThanks,\n\nDenton\n\n> \n> Thanks.\n"},{"id":"386653","messageId":"cover.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574207673.git.liu.denton@gmail.com","subject":"[PATCH v3 00/10] range-diff: learn `--notes`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:23Z","receivedAt":"2019-11-20T21:18:30Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"This patchset teaches range-diff and format-patch to pass `--notes`\noptions down to the `git log` machinery. This should make the behaviour\nmore configurable for users who either don't want notes to be displayed\nor want multiple notes refs to be displayed.\n\nChanges since v2:\n\n* Changed --notes= to --notes=\n\n* Added \"t3206: disable parameter substitution in heredoc\"\n\n* Updated the commit message of \"range-diff: pass through --notes to `git log`\"\n\nChanges since v1:\n\n* Complete overhaul of approach\n\nDenton Liu (10):\n argv-array: add space after `while`\n rev-list-options.txt: remove reference to --show-notes\n pretty-options.txt: --notes accepts a ref instead of treeish\n t3206: remove spaces after redirect operators\n t3206: disable parameter substitution in heredoc\n t3206: s/expected/expect/\n t3206: range-diff compares logs with commit notes\n range-diff: output `## Notes ##` header\n range-diff: pass through --notes to `git log`\n format-patch: pass notes configuration to range-diff\n\n Documentation/git-range-diff.txt | 6 +-\n Documentation/pretty-options.txt | 8 +-\n Documentation/rev-list-options.txt | 2 +-\n argv-array.c | 2 +-\n builtin/log.c | 24 ++-\n builtin/range-diff.c | 6 +-\n log-tree.c | 2 +-\n range-diff.c | 21 ++-\n range-diff.h | 4 +-\n t/t3206-range-diff.sh | 266 +++++++++++++++++++++++++----\n 10 files changed, 291 insertions(+), 50 deletions(-)\n\nRange-diff against v2:\n 1: 396ac06b0d = 1: fd78742570 argv-array: add space after `while`\n 2: 8cc7c8fe72 = 2: e1b023a6fc rev-list-options.txt: remove reference to --show-notes\n -: ---------- > 3: 4989956f12 pretty-options.txt: --notes accepts a ref instead of treeish\n 3: 22f5f07ace = 4: 85fcacf3f9 t3206: remove spaces after redirect operators\n -: ---------- > 5: 855a3df542 t3206: disable parameter substitution in heredoc\n 4: 922db36e7e = 6: 92df18b261 t3206: s/expected/expect/\n 5: d8ecda2d5e ! 7: 093d32ac4f t3206: demonstrate current notes behavior\n @@ Metadata\n Author: Denton Liu \n \n ## Commit message ##\n - t3206: demonstrate current notes behavior\n + t3206: range-diff compares logs with commit notes\n \n The test suite had a blindspot where it did not check the behavior of\n range-diff and format-patch when notes were present. Cover this\n 6: 7dd0b93b0b = 8: 2d1c849ecc range-diff: output `## Notes ##` header\n 7: 587a02a39c ! 9: 9c144e14c5 range-diff: passthrough --[no-]notes to `git log`\n @@ Metadata\n Author: Denton Liu \n \n ## Commit message ##\n - range-diff: passthrough --[no-]notes to `git log`\n + range-diff: pass through --notes to `git log`\n \n When a commit being range-diff'd has a note attached to it, the note\n will be compared as well. However, if a user has multiple notes refs or\n if they want to suppress notes from being printed, there is currently no\n way to do this.\n \n - Passthrough `---no--notes` to the `git log` call so that this option is\n - customizable.\n + Pass through `--[no-]notes[=]` to the `git log` call so that this\n + option is customizable.\n \n ## Documentation/git-range-diff.txt ##\n @@ Documentation/git-range-diff.txt: to revert to color all lines according to the outer diff markers\n \tSee the ``Algorithm`` section below for an explanation why this is\n \tneeded.\n \n -+--[no-]notes[=]::\n ++--[no-]notes[=]::\n +\tThis flag is passed to the `git log` program\n +\t(see linkgit:git-log[1]) that generates the patches.\n +\n 8: ce8cff7d0c = 10: 86318b3fe7 format-patch: pass notes configuration to range-diff\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386654","messageId":"fd78742570bbeaa4096eb0cce24fe33054aed176.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 01/10] argv-array: add space after `while`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:26Z","receivedAt":"2019-11-20T21:18:31Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n argv-array.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/argv-array.c b/argv-array.c\nindex f352ea9357..61ef8c0dfd 100644\n--- a/argv-array.c\n+++ b/argv-array.c\n@@ -46,7 +46,7 @@ void argv_array_pushl(struct argv_array *array, ...)\n \tconst char *arg;\n \n \tva_start(ap, array);\n-\twhile((arg = va_arg(ap, const char *)))\n+\twhile ((arg = va_arg(ap, const char *)))\n \t\targv_array_push(array, arg);\n \tva_end(ap);\n }\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386655","messageId":"e1b023a6fc2818f871e3033bd21f72a2de7b7a21.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 02/10] rev-list-options.txt: remove reference to --show-notes","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:28Z","receivedAt":"2019-11-20T21:18:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In ab18b2c0df (\"log/pretty-options: Document --[no-]notes and deprecate\nold notes options\", 2011-03-30), the `--show-notes` option was\ndeprecated. However, this reference to it still remains. Change it to\nreference the replacement option: `--notes`.\n\nSigned-off-by: Denton Liu \n---\n Documentation/rev-list-options.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 90ff9e2bea..311bc06a9b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -58,7 +58,7 @@ endif::git-rev-list[]\n \t`--all-match`).\n ifndef::git-rev-list[]\n +\n-When `--show-notes` is in effect, the message from the notes is\n+When `--notes` is in effect, the message from the notes is\n matched as if it were part of the log message.\n endif::git-rev-list[]\n \n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386656","messageId":"4989956f128fe36f743c25aacc89aae14be845ff.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 03/10] pretty-options.txt: --notes accepts a ref instead of treeish","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:31Z","receivedAt":"2019-11-20T21:18:37Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Although `--notes=` accepts and handles a tree-ish just fine, it isn't a\ncommon use-case for users to pass in bare trees. By having \"treeish\", it\nmakes it harder to click in users' minds that the argument here is the\nsame type as the `notes.displayRef` configuration variable, for example.\n\nChange `treeish` to `ref` so that it will be easier for users to make\nthis connection.\n\nNote that we don't completely lose the notion that `--notes=` can accept\na tree-ish. In git-notes.txt, we have\n\n\tIt is also permitted for a notes ref to point directly to a tree\n\tobject, in which case the history of the notes can be read with\n\t`git log -p -g `.\n\nwhich means that a hardcore user who wants to take advantage of this\nobscure use-case will be able to infer the connection and not be\ncompletely left in the dark.\n\nSigned-off-by: Denton Liu \n---\n Documentation/pretty-options.txt | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex e44fc8f738..6893a4a7ba 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -57,7 +57,7 @@ message by 4 spaces (i.e. 'medium', which is the default, 'full',\n and 'fuller').\n \n ifndef::git-rev-list[]\n---notes[=]::\n+--notes[=]::\n \tShow the notes (see linkgit:git-notes[1]) that annotate the\n \tcommit, when showing the commit log message. This is the default\n \tfor `git log`, `git show` and `git whatchanged` commands when\n@@ -68,8 +68,8 @@ By default, the notes shown are from the notes refs listed in the\n `core.notesRef` and `notes.displayRef` variables (or corresponding\n environment overrides). See linkgit:git-config[1] for more details.\n +\n-With an optional '' argument, use the treeish to find the notes\n-to display. The treeish can specify the full refname when it begins\n+With an optional '' argument, use the ref to find the notes\n+to display. The ref can specify the full refname when it begins\n with `refs/notes/`; when it begins with `notes/`, `refs/` and otherwise\n `refs/notes/` is prefixed to form a full name of the ref.\n +\n@@ -85,7 +85,7 @@ being displayed. Examples: \"--notes=foo\" will show only notes from\n \t\"--notes --notes=foo --no-notes --notes=bar\" will only show notes\n \tfrom \"refs/notes/bar\".\n \n---show-notes[=]::\n+--show-notes[=]::\n --[no-]standard-notes::\n \tThese options are deprecated. Use the above --notes/--no-notes\n \toptions instead.\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386657","messageId":"85fcacf3f90454900bbc7983fee4cba82ac02651.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 04/10] t3206: remove spaces after redirect operators","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:33Z","receivedAt":"2019-11-20T21:18:38Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For shell scripts, the usual convention is for there to be no space\nafter redirection operators, (e.g. `>file`, not `> file`). Remove the\none instance of this.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 0579cd9969..87c6c029db 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -8,7 +8,7 @@ test_description='range-diff tests'\n # harm than good. We need some real history.\n \n test_expect_success 'setup' '\n-\tgit fast-import < \"$TEST_DIRECTORY\"/t3206/history.export &&\n+\tgit fast-import <\"$TEST_DIRECTORY\"/t3206/history.export &&\n \ttest_oid_cache <<-EOF\n \t# topic\n \tt1 sha1:4de457d\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386658","messageId":"855a3df542a63ff77bd7164a4a7aab6d3b0fa447.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 05/10] t3206: disable parameter substitution in heredoc","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:36Z","receivedAt":"2019-11-20T21:18:40Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the first heredoc, parameter substitution is not used so prevent it\nfrom happening in the future (perhaps by accident) by escaping the limit\nEOF.\n\nThe remaining heredocs use parameter substitution so they cannot be\nchanged.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 87c6c029db..41606775d4 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -9,7 +9,7 @@ test_description='range-diff tests'\n \n test_expect_success 'setup' '\n \tgit fast-import <\"$TEST_DIRECTORY\"/t3206/history.export &&\n-\ttest_oid_cache <<-EOF\n+\ttest_oid_cache <<-\\EOF\n \t# topic\n \tt1 sha1:4de457d\n \tt2 sha1:fccce22\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386659","messageId":"92df18b261ce9299a5f76ab61c93ffeb0503521b.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 06/10] t3206: s/expected/expect/","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:38Z","receivedAt":"2019-11-20T21:18:44Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"For test cases, the usual convention is to name expected output files\n\"expect\", not \"expected\". Replace all instances of \"expected\" with\n\"expect\".\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 64 +++++++++++++++++++++----------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 41606775d4..f654aed54f 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -121,88 +121,88 @@ test_expect_success 'setup' '\n test_expect_success 'simple A..B A..C (unmodified)' '\n \tgit range-diff --no-color master..topic master..unmodified \\\n \t\t>actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'simple B...C (unmodified)' '\n \tgit range-diff --no-color topic...unmodified >actual &&\n-\t# same \"expected\" as above\n-\ttest_cmp expected actual\n+\t# same \"expect\" as above\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'simple A B C (unmodified)' '\n \tgit range-diff --no-color master topic unmodified >actual &&\n-\t# same \"expected\" as above\n-\ttest_cmp expected actual\n+\t# same \"expect\" as above\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid r1) s/5/A/\n \t3: $(test_oid t3) = 2: $(test_oid r2) s/11/B/\n \t4: $(test_oid t4) = 3: $(test_oid r3) s/12/B/\n \t2: $(test_oid t2) = 4: $(test_oid r4) s/4/A/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'removed a commit' '\n \tgit range-diff --no-color master topic removed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid d1) s/5/A/\n \t2: $(test_oid t2) < -: $(test_oid __) s/4/A/\n \t3: $(test_oid t3) = 2: $(test_oid d2) s/11/B/\n \t4: $(test_oid t4) = 3: $(test_oid d3) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'added a commit' '\n \tgit range-diff --no-color master topic added >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid a1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid a2) s/4/A/\n \t-: $(test_oid __) > 3: $(test_oid a3) s/6/A/\n \t3: $(test_oid t3) = 4: $(test_oid a4) s/11/B/\n \t4: $(test_oid t4) = 5: $(test_oid a5) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'new base, A B C' '\n \tgit range-diff --no-color master topic rebased >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid b1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid b2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid b3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid b4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'new base, B...C' '\n \t# this syntax includes the commits from master!\n \tgit range-diff --no-color topic...rebased >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t-: $(test_oid __) > 1: $(test_oid b5) unrelated\n \t1: $(test_oid t1) = 2: $(test_oid b1) s/5/A/\n \t2: $(test_oid t2) = 3: $(test_oid b2) s/4/A/\n \t3: $(test_oid t3) = 4: $(test_oid b3) s/11/B/\n \t4: $(test_oid t4) = 5: $(test_oid b4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit' '\n \tgit range-diff --no-color topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n@@ -226,23 +226,23 @@ test_expect_success 'changed commit' '\n \t +B\n \t 13\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with --no-patch diff option' '\n \tgit range-diff --no-color --no-patch topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n \t4: $(test_oid t4) ! 4: $(test_oid c4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with --stat diff option' '\n \tgit range-diff --no-color --stat topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t a => b | 0\n \t 1 file changed, 0 insertions(+), 0 deletions(-)\n@@ -256,12 +256,12 @@ test_expect_success 'changed commit with --stat diff option' '\n \t a => b | 0\n \t 1 file changed, 0 insertions(+), 0 deletions(-)\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'changed commit with sm config' '\n \tgit range-diff --no-color --submodule=log topic...changed >actual &&\n-\tcat >expected <<-EOF &&\n+\tcat >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/\n \t2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/\n \t3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/\n@@ -285,12 +285,12 @@ test_expect_success 'changed commit with sm config' '\n \t +B\n \t 13\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'renamed file' '\n \tgit range-diff --no-color --submodule=log topic...renamed-file >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid n1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid n2) s/4/A/\n \t @@ Metadata\n@@ -330,12 +330,12 @@ test_expect_success 'renamed file' '\n \t Z 10\n \t Z B\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'file with mode only change' '\n \tgit range-diff --no-color --submodule=log topic...mode-only-change >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: fccce22 ! 1: 4d39cb3 s/4/A/\n \t @@ Metadata\n \t ZAuthor: Thomas Rast \n@@ -370,12 +370,12 @@ test_expect_success 'file with mode only change' '\n \t + ## other-file (mode change 100644 => 100755) ##\n \t3: a63e992 = 3: 4c1e0f5 s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'file added and later removed' '\n \tgit range-diff --no-color --submodule=log topic...added-removed >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid s1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid s2) s/4/A/\n \t @@ Metadata\n@@ -411,7 +411,7 @@ test_expect_success 'file added and later removed' '\n \t + ## new-file (deleted) ##\n \t4: $(test_oid t4) = 4: $(test_oid s4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'no commits on one side' '\n@@ -421,7 +421,7 @@ test_expect_success 'no commits on one side' '\n \n test_expect_success 'changed message' '\n \tgit range-diff --no-color topic...changed-message >actual &&\n-\tsed s/Z/\\ /g >expected <<-EOF &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n \t1: $(test_oid t1) = 1: $(test_oid m1) s/5/A/\n \t2: $(test_oid t2) ! 2: $(test_oid m2) s/4/A/\n \t @@ Metadata\n@@ -436,7 +436,7 @@ test_expect_success 'changed message' '\n \t3: $(test_oid t3) = 3: $(test_oid m3) s/11/B/\n \t4: $(test_oid t4) = 4: $(test_oid m4) s/12/B/\n \tEOF\n-\ttest_cmp expected actual\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'dual-coloring' '\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386660","messageId":"093d32ac4f198341232a46bf867317b8c3c7d566.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 07/10] t3206: range-diff compares logs with commit notes","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:41Z","receivedAt":"2019-11-20T21:18:46Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test suite had a blindspot where it did not check the behavior of\nrange-diff and format-patch when notes were present. Cover this\nblindspot.\n\nSigned-off-by: Denton Liu \n---\n t/t3206-range-diff.sh | 52 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 52 insertions(+)\n\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex f654aed54f..19ba644933 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -505,4 +505,56 @@ test_expect_success 'range-diff overrides diff.noprefix internally' '\n \tgit -c diff.noprefix=true range-diff HEAD^...\n '\n \n+test_expect_success 'range-diff compares notes by default' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit range-diff --no-color master..topic master..unmodified \\\n+\t\t>actual &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n+\t @@ Metadata\n+\t Z\n+\t Z ## Commit message ##\n+\t Z s/12/B/\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'format-patch --range-diff compares notes by default' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Metadata\n+\t Z\n+\t Z ## Commit message ##\n+\t Z s/12/B/\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Metadata/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386661","messageId":"2d1c849ecc30c14ace8a99753c35c96d168347bc.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 08/10] range-diff: output `## Notes ##` header","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:43Z","receivedAt":"2019-11-20T21:18:49Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When notes were included in the output of range-diff, they were just\nmashed together with the rest of the commit message. As a result, users\nwouldn't be able to clearly distinguish where the commit message ended\nand where the notes started.\n\nOutput a `## Notes ##` header when notes are detected so that notes can\nbe compared more clearly.\n\nNote that we handle case of `Notes (): -> ## Notes () ##` with\nthis code as well. We can't test this in this patch, however, since\nthere is currently no way to pass along different notes refs to `git\nlog`. This will be fixed in a future patch.\n\nSigned-off-by: Denton Liu \n---\n range-diff.c | 6 ++++++\n t/t3206-range-diff.sh | 14 +++++++-------\n 2 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 7fed5a3b4b..623397221d 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -144,6 +144,12 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t\tstrbuf_addstr(&buf, line);\n \t\t\t\tstrbuf_addstr(&buf, \"\\n\\n\");\n \t\t\t\tstrbuf_addstr(&buf, \" ## Commit message ##\\n\");\n+\t\t\t} else if (starts_with(line, \"Notes\") &&\n+\t\t\t\t line[strlen(line) - 1] == ':') {\n+\t\t\t\tstrbuf_addstr(&buf, \"\\n\\n\");\n+\t\t\t\t/* strip the trailing colon */\n+\t\t\t\tstrbuf_addf(&buf, \" ## %.*s ##\\n\",\n+\t\t\t\t\t (int)(strlen(line) - 1), line);\n \t\t\t} else if (starts_with(line, \" \")) {\n \t\t\t\tp = line + len - 2;\n \t\t\t\twhile (isspace(*p) && p >= line)\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 19ba644933..b936c16dd1 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -516,10 +516,10 @@ test_expect_success 'range-diff compares notes by default' '\n \t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n \t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n \t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n-\t @@ Metadata\n+\t @@ Commit message\n \t Z\n-\t Z ## Commit message ##\n-\t Z s/12/B/\n+\t Z\n+\t Z ## Notes ##\n \t - topic note\n \t + unmodified note\n \t Z\n@@ -543,17 +543,17 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgrep \"= 3: .* s/11/B\" 0000-* &&\n \tgrep \"! 4: .* s/12/B\" 0000-* &&\n \tsed s/Z/\\ /g >expect <<-EOF &&\n-\t @@ Metadata\n+\t @@ Commit message\n \t Z\n-\t Z ## Commit message ##\n-\t Z s/12/B/\n+\t Z\n+\t Z ## Notes ##\n \t - topic note\n \t + unmodified note\n \t Z\n \t Z ## file ##\n \t Z@@ file: A\n \tEOF\n-\tsed \"/@@ Metadata/,/@@ file: A/!d\" 0000-* >actual &&\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386662","messageId":"9c144e14c52ddc5ea3c280d51124fbf56c54c215.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 09/10] range-diff: pass through --notes to `git log`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:45Z","receivedAt":"2019-11-20T21:18:51Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When a commit being range-diff'd has a note attached to it, the note\nwill be compared as well. However, if a user has multiple notes refs or\nif they want to suppress notes from being printed, there is currently no\nway to do this.\n\nPass through `--[no-]notes[=]` to the `git log` call so that this\noption is customizable.\n\nSigned-off-by: Denton Liu \n---\n Documentation/git-range-diff.txt | 6 +++-\n builtin/log.c | 2 +-\n builtin/range-diff.c | 6 +++-\n log-tree.c | 2 +-\n range-diff.c | 15 ++++++----\n range-diff.h | 4 ++-\n t/t3206-range-diff.sh | 47 ++++++++++++++++++++++++++++++++\n 7 files changed, 72 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 8a6ea2c6c5..a2c25f4490 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -57,6 +57,10 @@ to revert to color all lines according to the outer diff markers\n \tSee the ``Algorithm`` section below for an explanation why this is\n \tneeded.\n \n+--[no-]notes[=]::\n+\tThis flag is passed to the `git log` program\n+\t(see linkgit:git-log[1]) that generates the patches.\n+\n ::\n \tCompare the commits specified by the two ranges, where\n \t`` is considered an older version of ``.\n@@ -75,7 +79,7 @@ to revert to color all lines according to the outer diff markers\n linkgit:git-diff[1]), most notably the `--color=[]` and\n `--no-color` options. These options are used when generating the \"diff\n between patches\", i.e. to compare the author, commit message and diff of\n-corresponding old/new commits. There is currently no means to tweak the\n+corresponding old/new commits. There is currently no means to tweak most of the\n diff options passed to `git log` when generating those patches.\n \n OUTPUT STABILITY\ndiff --git a/builtin/log.c b/builtin/log.c\nindex a26f223ab4..047ac4594d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1189,7 +1189,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n \t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts);\n+\t\t\t\trev->creation_factor, 1, &opts, NULL);\n \t}\n }\n \ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 9202e75544..98acf3533e 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -15,12 +15,16 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n \tstruct diff_options diffopt = { NULL };\n+\tstruct argv_array other_arg = ARGV_ARRAY_INIT;\n \tint simple_color = -1;\n \tstruct option range_diff_options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n \t\t\t N_(\"use simple diff colors\")),\n+\t\tOPT_PASSTHRU_ARGV(0, \"notes\", &other_arg,\n+\t\t\t\t N_(\"notes\"), N_(\"passed to 'git log'\"),\n+\t\t\t\t PARSE_OPT_OPTARG),\n \t\tOPT_END()\n \t};\n \tstruct option *options;\n@@ -78,7 +82,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \tFREE_AND_NULL(options);\n \n \tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n-\t\t\t simple_color < 1, &diffopt);\n+\t\t\t simple_color < 1, &diffopt, &other_arg);\n \n \tstrbuf_release(&range1);\n \tstrbuf_release(&range2);\ndiff --git a/log-tree.c b/log-tree.c\nindex 923a299e70..151e12f415 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -770,7 +770,7 @@ void show_log(struct rev_info *opt)\n \t\topts.use_color = opt->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n-\t\t\t\topt->creation_factor, 1, &opts);\n+\t\t\t\topt->creation_factor, 1, &opts, NULL);\n \n \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n \t}\ndiff --git a/range-diff.c b/range-diff.c\nindex 623397221d..f56b4012a2 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -40,7 +40,8 @@ static size_t find_end_of_line(char *buffer, unsigned long size)\n * Reads the patches into a string list, with the `util` field being populated\n * as struct object_id (will need to be free()d).\n */\n-static int read_patches(const char *range, struct string_list *list)\n+static int read_patches(const char *range, struct string_list *list,\n+\t\t\tstruct argv_array *other_arg)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT;\n@@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t\"--output-indicator-new=>\",\n \t\t\t\"--output-indicator-old=<\",\n \t\t\t\"--output-indicator-context=#\",\n-\t\t\t\"--no-abbrev-commit\", range,\n+\t\t\t\"--no-abbrev-commit\",\n \t\t\tNULL);\n+\tif (other_arg)\n+\t\targv_array_pushv(&cp.args, other_arg->argv);\n+\targv_array_push(&cp.args, range);\n \tcp.out = -1;\n \tcp.no_stdin = 1;\n \tcp.git_cmd = 1;\n@@ -502,16 +506,17 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n \n int show_range_diff(const char *range1, const char *range2,\n \t\t int creation_factor, int dual_color,\n-\t\t struct diff_options *diffopt)\n+\t\t struct diff_options *diffopt,\n+\t\t struct argv_array *other_arg)\n {\n \tint res = 0;\n \n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1))\n+\tif (read_patches(range1, &branch1, other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n-\tif (!res && read_patches(range2, &branch2))\n+\tif (!res && read_patches(range2, &branch2, other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range2);\n \n \tif (!res) {\ndiff --git a/range-diff.h b/range-diff.h\nindex 08a50b6e98..7d918ab9ed 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -2,6 +2,7 @@\n #define RANGE_DIFF_H\n \n #include \"diff.h\"\n+#include \"argv-array.h\"\n \n #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n \n@@ -12,6 +13,7 @@\n */\n int show_range_diff(const char *range1, const char *range2,\n \t\t int creation_factor, int dual_color,\n-\t\t struct diff_options *diffopt);\n+\t\t struct diff_options *diffopt,\n+\t\t struct argv_array *other_arg);\n \n #endif\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex b936c16dd1..521b4a83ec 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -529,6 +529,53 @@ test_expect_success 'range-diff compares notes by default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'range-diff with --no-notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit range-diff --no-color --no-notes master..topic master..unmodified \\\n+\t\t>actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'range-diff with multiple --notes' '\n+\tgit notes --ref=note1 add -m \"topic note1\" topic &&\n+\tgit notes --ref=note1 add -m \"unmodified note1\" unmodified &&\n+\ttest_when_finished git notes --ref=note1 remove topic unmodified &&\n+\tgit notes --ref=note2 add -m \"topic note2\" topic &&\n+\tgit notes --ref=note2 add -m \"unmodified note2\" unmodified &&\n+\ttest_when_finished git notes --ref=note2 remove topic unmodified &&\n+\tgit range-diff --no-color --notes=note1 --notes=note2 master..topic master..unmodified \\\n+\t\t>actual &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/\n+\t2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/\n+\t3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/\n+\t4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes (note1) ##\n+\t - topic note1\n+\t + unmodified note1\n+\t Z\n+\t Z\n+\t Z ## Notes (note2) ##\n+\t - topic note2\n+\t + unmodified note2\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386663","messageId":"86318b3fe7b53f5bad0f0094bc237afc74108ecc.1574284470.git.liu.denton@gmail.com","threadId":"52288","inReplyTo":"cover.1574284470.git.liu.denton@gmail.com","subject":"[PATCH v3 10/10] format-patch: pass notes configuration to range-diff","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-20T21:18:47Z","receivedAt":"2019-11-20T21:18:54Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Since format-patch accepts `--[no-]notes`, one would expect the\nrange-diff generated to also respect the setting. Unfortunately, the\nrange-diff we currently generate only uses the default option (which\nalways outputs default notes, even when notes are not being used\nelsewhere).\n\nPass the notes configuration to range-diff so that it can honor it.\n\nSigned-off-by: Denton Liu \n---\n builtin/log.c | 24 +++++++++-\n t/t3206-range-diff.sh | 101 +++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 123 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 047ac4594d..e192f219d9 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1111,6 +1111,25 @@ static void prepare_cover_text(struct pretty_print_context *pp,\n \tstrbuf_release(&subject_sb);\n }\n \n+static int get_notes_refs(struct string_list_item *item, void *arg)\n+{\n+\targv_array_pushf(arg, \"--notes=%s\", item->string);\n+\treturn 0;\n+}\n+\n+static void get_notes_args(struct argv_array *arg, struct rev_info *rev)\n+{\n+\tif (!rev->show_notes) {\n+\t\targv_array_push(arg, \"--no-notes\");\n+\t} else if (rev->notes_opt.use_default_notes > 0 ||\n+\t\t (rev->notes_opt.use_default_notes == -1 &&\n+\t\t !rev->notes_opt.extra_notes_refs.nr)) {\n+\t\targv_array_push(arg, \"--notes\");\n+\t} else {\n+\t\tfor_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg);\n+\t}\n+}\n+\n static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n@@ -1183,13 +1202,16 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t * can be added later if deemed desirable.\n \t\t */\n \t\tstruct diff_options opts;\n+\t\tstruct argv_array other_arg = ARGV_ARRAY_INIT;\n \t\tdiff_setup(&opts);\n \t\topts.file = rev->diffopt.file;\n \t\topts.use_color = rev->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n+\t\tget_notes_args(&other_arg, rev);\n \t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts, NULL);\n+\t\t\t\trev->creation_factor, 1, &opts, &other_arg);\n+\t\targv_array_clear(&other_arg);\n \t}\n }\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 521b4a83ec..ec2b456dbb 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -576,7 +576,7 @@ test_expect_success 'range-diff with multiple --notes' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'format-patch --range-diff compares notes by default' '\n+test_expect_success 'format-patch --range-diff does not compare notes by default' '\n \tgit notes add -m \"topic note\" topic &&\n \tgit notes add -m \"unmodified note\" unmodified &&\n \ttest_when_finished git notes remove topic unmodified &&\n@@ -588,6 +588,40 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \tgrep \"= 1: .* s/5/A\" 0000-* &&\n \tgrep \"= 2: .* s/4/A\" 0000-* &&\n \tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"= 4: .* s/12/B\" 0000-* &&\n+\t! grep \"Notes\" 0000-* &&\n+\t! grep \"note\" 0000-*\n+'\n+\n+test_expect_success 'format-patch --range-diff with --no-notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --no-notes --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"= 4: .* s/12/B\" 0000-* &&\n+\t! grep \"Notes\" 0000-* &&\n+\t! grep \"note\" 0000-*\n+'\n+\n+test_expect_success 'format-patch --range-diff with --notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\tgit format-patch --notes --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n \tgrep \"! 4: .* s/12/B\" 0000-* &&\n \tsed s/Z/\\ /g >expect <<-EOF &&\n \t @@ Commit message\n@@ -604,4 +638,69 @@ test_expect_success 'format-patch --range-diff compares notes by default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch --range-diff with --notes' '\n+\tgit notes add -m \"topic note\" topic &&\n+\tgit notes add -m \"unmodified note\" unmodified &&\n+\ttest_when_finished git notes remove topic unmodified &&\n+\ttest_config format.notes true &&\n+\tgit format-patch --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes ##\n+\t - topic note\n+\t + unmodified note\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'format-patch --range-diff with multiple notes' '\n+\tgit notes --ref=note1 add -m \"topic note1\" topic &&\n+\tgit notes --ref=note1 add -m \"unmodified note1\" unmodified &&\n+\ttest_when_finished git notes --ref=note1 remove topic unmodified &&\n+\tgit notes --ref=note2 add -m \"topic note2\" topic &&\n+\tgit notes --ref=note2 add -m \"unmodified note2\" unmodified &&\n+\ttest_when_finished git notes --ref=note2 remove topic unmodified &&\n+\tgit format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \\\n+\t\tmaster..unmodified >actual &&\n+\ttest_when_finished \"rm 000?-*\" &&\n+\ttest_line_count = 5 actual &&\n+\ttest_i18ngrep \"^Range-diff:$\" 0000-* &&\n+\tgrep \"= 1: .* s/5/A\" 0000-* &&\n+\tgrep \"= 2: .* s/4/A\" 0000-* &&\n+\tgrep \"= 3: .* s/11/B\" 0000-* &&\n+\tgrep \"! 4: .* s/12/B\" 0000-* &&\n+\tsed s/Z/\\ /g >expect <<-EOF &&\n+\t @@ Commit message\n+\t Z\n+\t Z\n+\t Z ## Notes (note1) ##\n+\t - topic note1\n+\t + unmodified note1\n+\t Z\n+\t Z\n+\t Z ## Notes (note2) ##\n+\t - topic note2\n+\t + unmodified note2\n+\t Z\n+\t Z ## file ##\n+\t Z@@ file: A\n+\tEOF\n+\tsed \"/@@ Commit message/,/@@ file: A/!d\" 0000-* >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.24.0.450.g7a9a4598a9\n\n"},{"id":"386716","messageId":"CAPig+cQ0xO2ya=3ok=GPV2Ji2HJSFvCtokiRpWJhBAR1skqKVQ@mail.gmail.com","threadId":"52288","inReplyTo":"20191120191258.GA73969@generichostname","subject":"Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-11-21T12:43:18Z","receivedAt":"2019-11-21T12:43:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Nov 20, 2019 at 2:13 PM Denton Liu wrote:\n> On Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote:\n> > Denton Liu writes:\n> > > +#include \"argv-array.h\"\n> > >\n> > > int show_range_diff(const char *range1, const char *range2,\n> > > int creation_factor, int dual_color,\n> > > - struct diff_options *diffopt);\n> > > + struct diff_options *diffopt,\n> > > + struct argv_array *other_arg);\n> >\n> > I thought a mere use of \"pointer to a struct\" did not have to bring\n> > the definition of the struct into the picture? In other words,\n> > wouldn't it be fine to leave the other_arg a pointer to an opaque\n> > structure by not including \"argv-array.h\" in this file?\n>\n> Without including \"argv-array.h\", we get the following hdr-check error:\n>\n> $ make range-diff.hco\n> In file included from range-diff.hcc:2:\n> ./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility]\n> struct argv_array *other_arg);\n\nDid you forward-declare 'argv_array' in range-diff.h? That is, rather than:\n\n #include \"argv-array.h\"\n\ninstead say:\n\n struct argv_array;\n\nwhich tells the compiler that the type exists, thus allowing you to\ndeal with pointers to the struct, as long as you don't try to access\nany of the struct's members in code which has seen only the forward\ndeclaration. You would still need to #include \"argv-array.h\" in\nrange-diff.c, though.\n"},{"id":"386738","messageId":"20191121183539.GA99414@generichostname","threadId":"52288","inReplyTo":"CAPig+cQ0xO2ya=3ok=GPV2Ji2HJSFvCtokiRpWJhBAR1skqKVQ@mail.gmail.com","subject":"Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-11-21T18:35:39Z","receivedAt":"2019-11-21T18:35:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Eric,\n\nOn Thu, Nov 21, 2019 at 07:43:18AM -0500, Eric Sunshine wrote:\n> On Wed, Nov 20, 2019 at 2:13 PM Denton Liu wrote:\n> > On Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote:\n> > > Denton Liu writes:\n> > > > +#include \"argv-array.h\"\n> > > >\n> > > > int show_range_diff(const char *range1, const char *range2,\n> > > > int creation_factor, int dual_color,\n> > > > - struct diff_options *diffopt);\n> > > > + struct diff_options *diffopt,\n> > > > + struct argv_array *other_arg);\n> > >\n> > > I thought a mere use of \"pointer to a struct\" did not have to bring\n> > > the definition of the struct into the picture? In other words,\n> > > wouldn't it be fine to leave the other_arg a pointer to an opaque\n> > > structure by not including \"argv-array.h\" in this file?\n> >\n> > Without including \"argv-array.h\", we get the following hdr-check error:\n> >\n> > $ make range-diff.hco\n> > In file included from range-diff.hcc:2:\n> > ./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility]\n> > struct argv_array *other_arg);\n> \n> Did you forward-declare 'argv_array' in range-diff.h? That is, rather than:\n> \n> #include \"argv-array.h\"\n> \n> instead say:\n> \n> struct argv_array;\n\n*facepalm* Damn, sorry for the brainfart. I completely forgot to do\nthat.\n\nThat being said, I'd prefer to just include the header for consistency\nsince we already have the #include \"diff.h\" up there. Or alternatively,\nI could write a patch to change both into forward-declarations.\n\nPersonally, I prefer to avoid forward-declarations whenever possible\nbecause it adds duplication. If we have a strong preference for one or\nthe other, should we include it in the CodingGuidelines?\n\nThanks,\n\nDenton\n\n> \n> which tells the compiler that the type exists, thus allowing you to\n> deal with pointers to the struct, as long as you don't try to access\n> any of the struct's members in code which has seen only the forward\n> declaration. You would still need to #include \"argv-array.h\" in\n> range-diff.c, though.\n"}]}