* [PATCH 0/3] range-diff: don't compare notes
@ 2019-11-19 1:06 Denton Liu
2019-11-19 1:06 ` [PATCH 1/3] t3206: remove spaces after redirect operators Denton Liu
` (4 more replies)
0 siblings, 5 replies; 32+ messages in thread
From: Denton Liu @ 2019-11-19 1:06 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer
When I was using range-diff at $DAYJOB earlier, I realised that it
includes commit notes as part of the commit message comparison. This is
undesired behaviour so this patchset documents it and stops it from
happening.
Denton Liu (3):
t3206: remove spaces after redirect operators
t3206: demonstrate failure with notes
range-diff: use --no-notes to generate patches
range-diff.c | 2 +-
t/t3206-range-diff.sh | 17 ++++++++++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
--
2.24.0.420.g9ac4901264
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 1/3] t3206: remove spaces after redirect operators 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu @ 2019-11-19 1:06 ` Denton Liu 2019-11-19 1:06 ` [PATCH 2/3] t3206: demonstrate failure with notes Denton Liu ` (3 subsequent siblings) 4 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 1:06 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer For shell scripts, the usual convention is for there to be no space after redirection operators, (e.g. `>file`, not `> file`). Remove the one instance of this. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 0579cd9969..87c6c029db 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -8,7 +8,7 @@ test_description='range-diff tests' # harm than good. We need some real history. test_expect_success 'setup' ' - git fast-import < "$TEST_DIRECTORY"/t3206/history.export && + git fast-import <"$TEST_DIRECTORY"/t3206/history.export && test_oid_cache <<-EOF # topic t1 sha1:4de457d -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/3] t3206: demonstrate failure with notes 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu 2019-11-19 1:06 ` [PATCH 1/3] t3206: remove spaces after redirect operators Denton Liu @ 2019-11-19 1:06 ` Denton Liu 2019-11-19 3:03 ` Junio C Hamano 2019-11-19 1:06 ` [PATCH 3/3] range-diff: use --no-notes to generate patches Denton Liu ` (2 subsequent siblings) 4 siblings, 1 reply; 32+ messages in thread From: Denton Liu @ 2019-11-19 1:06 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer When a commit being range-diff'd has a note attached to it, the note will be compared as well. However, this shouldn't happen since the purpose of range-diff is to compare the difference between two commit-ranges and, since the note attached to a commit is mutable, they shouldn't be included as part of the comparison. Demonstrate this failure in t3206. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 87c6c029db..29b9a6f854 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -505,4 +505,19 @@ test_expect_success 'range-diff overrides diff.noprefix internally' ' git -c diff.noprefix=true range-diff HEAD^... ' +test_expect_failure 'range-diff does not compare notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git range-diff --no-color master..topic master..unmodified \ + >actual && + cat >expected <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/ + EOF + test_cmp expected actual +' + test_done -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] t3206: demonstrate failure with notes 2019-11-19 1:06 ` [PATCH 2/3] t3206: demonstrate failure with notes Denton Liu @ 2019-11-19 3:03 ` Junio C Hamano 0 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2019-11-19 3:03 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer Denton Liu <liu.denton@gmail.com> writes: > When a commit being range-diff'd has a note attached to it, the note > will be compared as well. However, this shouldn't happen since the > purpose of range-diff is to compare the difference between two > commit-ranges and, since the note attached to a commit is mutable, they > shouldn't be included as part of the comparison. I do not agree with the reasoning. Commits in the new iteration of the same series would have note attached to them that are different from the note attached to corresponding commits in the old iteration. Imagine that "git am" gets enhanced to store the per-iteration comments you write under the three-dash lines (e.g. "fixed typo in the log message pointed out by X") as notes attached to each of the resulting commits. It does make sense to compare the commits _with_ notes by default, if these notes are configured to be shown in "git show" output by default for the user. I do think that it may be a good idea to optionally allow comparison without notes, when "--no-notes" is given to range-diff on the command line, by passing the option through, though. Thanks. ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/3] range-diff: use --no-notes to generate patches 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu 2019-11-19 1:06 ` [PATCH 1/3] t3206: remove spaces after redirect operators Denton Liu 2019-11-19 1:06 ` [PATCH 2/3] t3206: demonstrate failure with notes Denton Liu @ 2019-11-19 1:06 ` Denton Liu 2019-11-19 2:56 ` [PATCH 0/3] range-diff: don't compare notes Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu 4 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 1:06 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer When a commit being range-diff'd has a note attached to it, the note will be compared as well. However, this shouldn't happen since the purpose of range-diff is to compare the difference between two commit-ranges and, since the note attached to a commit is mutable, they shouldn't be included as part of the comparison. Add `--no-notes` to the `git log` call and mark the corresponding test as passing. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- range-diff.c | 2 +- t/t3206-range-diff.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/range-diff.c b/range-diff.c index 7fed5a3b4b..725930ee92 100644 --- a/range-diff.c +++ b/range-diff.c @@ -52,7 +52,7 @@ static int read_patches(const char *range, struct string_list *list) argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges", "--reverse", "--date-order", "--decorate=no", - "--no-prefix", + "--no-prefix", "--no-notes", /* * Choose indicators that are not used anywhere * else in diffs, but still look reasonable diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 29b9a6f854..671703f85b 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -505,7 +505,7 @@ test_expect_success 'range-diff overrides diff.noprefix internally' ' git -c diff.noprefix=true range-diff HEAD^... ' -test_expect_failure 'range-diff does not compare notes' ' +test_expect_success 'range-diff does not compare notes' ' git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && test_when_finished git notes remove topic unmodified && -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 0/3] range-diff: don't compare notes 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu ` (2 preceding siblings ...) 2019-11-19 1:06 ` [PATCH 3/3] range-diff: use --no-notes to generate patches Denton Liu @ 2019-11-19 2:56 ` Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu 4 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2019-11-19 2:56 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer Denton Liu <liu.denton@gmail.com> writes: > When I was using range-diff at $DAYJOB earlier, I realised that it > includes commit notes as part of the commit message comparison. This is > undesired behaviour so this patchset documents it and stops it from > happening. I actually wish it allowed me to compare them _with_ --notes=<ref> specified by the user. If you are passing --no-notes through, perhaps you can also pass through such an option while at it. ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/8] range-diff: learn `--notes` 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu ` (3 preceding siblings ...) 2019-11-19 2:56 ` [PATCH 0/3] range-diff: don't compare notes Junio C Hamano @ 2019-11-19 23:55 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 1/8] argv-array: add space after `while` Denton Liu ` (8 more replies) 4 siblings, 9 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano This patchset teaches range-diff and format-patch to pass `--notes` options down to the `git log` machinery. This should make the behaviour more configurable for users who either don't want notes to be displayed or want multiple notes refs to be displayed. Changes since v1: * Complete overhaul of approach Denton Liu (8): argv-array: add space after `while` rev-list-options.txt: remove reference to --show-notes t3206: remove spaces after redirect operators t3206: s/expected/expect/ t3206: demonstrate current notes behavior range-diff: output `## Notes ##` header range-diff: passthrough --[no-]notes to `git log` format-patch: pass notes configuration to range-diff Documentation/git-range-diff.txt | 6 +- Documentation/rev-list-options.txt | 2 +- argv-array.c | 2 +- builtin/log.c | 24 ++- builtin/range-diff.c | 6 +- log-tree.c | 2 +- range-diff.c | 21 ++- range-diff.h | 4 +- t/t3206-range-diff.sh | 264 +++++++++++++++++++++++++---- 9 files changed, 286 insertions(+), 45 deletions(-) -- 2.24.0.420.g9ac4901264 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/8] argv-array: add space after `while` 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 2/8] rev-list-options.txt: remove reference to --show-notes Denton Liu ` (7 subsequent siblings) 8 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano Signed-off-by: Denton Liu <liu.denton@gmail.com> --- argv-array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/argv-array.c b/argv-array.c index f352ea9357..61ef8c0dfd 100644 --- a/argv-array.c +++ b/argv-array.c @@ -46,7 +46,7 @@ void argv_array_pushl(struct argv_array *array, ...) const char *arg; va_start(ap, array); - while((arg = va_arg(ap, const char *))) + while ((arg = va_arg(ap, const char *))) argv_array_push(array, arg); va_end(ap); } -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/8] rev-list-options.txt: remove reference to --show-notes 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu 2019-11-19 23:55 ` [PATCH v2 1/8] argv-array: add space after `while` Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 3/8] t3206: remove spaces after redirect operators Denton Liu ` (6 subsequent siblings) 8 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano In ab18b2c0df ("log/pretty-options: Document --[no-]notes and deprecate old notes options", 2011-03-30), the `--show-notes` option was deprecated. However, this reference to it still remains. Change it to reference the replacement option: `--notes`. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/rev-list-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 90ff9e2bea..311bc06a9b 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -58,7 +58,7 @@ endif::git-rev-list[] `--all-match`). ifndef::git-rev-list[] + -When `--show-notes` is in effect, the message from the notes is +When `--notes` is in effect, the message from the notes is matched as if it were part of the log message. endif::git-rev-list[] -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/8] t3206: remove spaces after redirect operators 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu 2019-11-19 23:55 ` [PATCH v2 1/8] argv-array: add space after `while` Denton Liu 2019-11-19 23:55 ` [PATCH v2 2/8] rev-list-options.txt: remove reference to --show-notes Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-20 0:20 ` Eric Sunshine 2019-11-19 23:55 ` [PATCH v2 4/8] t3206: s/expected/expect/ Denton Liu ` (5 subsequent siblings) 8 siblings, 1 reply; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano For shell scripts, the usual convention is for there to be no space after redirection operators, (e.g. `>file`, not `> file`). Remove the one instance of this. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 0579cd9969..87c6c029db 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -8,7 +8,7 @@ test_description='range-diff tests' # harm than good. We need some real history. test_expect_success 'setup' ' - git fast-import < "$TEST_DIRECTORY"/t3206/history.export && + git fast-import <"$TEST_DIRECTORY"/t3206/history.export && test_oid_cache <<-EOF # topic t1 sha1:4de457d -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] t3206: remove spaces after redirect operators 2019-11-19 23:55 ` [PATCH v2 3/8] t3206: remove spaces after redirect operators Denton Liu @ 2019-11-20 0:20 ` Eric Sunshine 0 siblings, 0 replies; 32+ messages in thread From: Eric Sunshine @ 2019-11-20 0:20 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer, Junio C Hamano On Tue, Nov 19, 2019 at 6:55 PM Denton Liu <liu.denton@gmail.com> wrote: > For shell scripts, the usual convention is for there to be no space > after redirection operators, (e.g. `>file`, not `> file`). Remove the > one instance of this. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > @@ -8,7 +8,7 @@ test_description='range-diff tests' > test_expect_success 'setup' ' > - git fast-import < "$TEST_DIRECTORY"/t3206/history.export && > + git fast-import <"$TEST_DIRECTORY"/t3206/history.export && > test_oid_cache <<-EOF > # topic > t1 sha1:4de457d While you're here cleaning style problems, you could also change this here-doc from -EOF to -\EOF. (This is the only here-doc in this script which can use -\EOF; all the others need -EOF.) ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 4/8] t3206: s/expected/expect/ 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (2 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 3/8] t3206: remove spaces after redirect operators Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 5/8] t3206: demonstrate current notes behavior Denton Liu ` (4 subsequent siblings) 8 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano For test cases, the usual convention is to name expected output files "expect", not "expected". Replace all instances of "expected" with "expect". Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 87c6c029db..e14b4951bb 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -121,88 +121,88 @@ test_expect_success 'setup' ' test_expect_success 'simple A..B A..C (unmodified)' ' git range-diff --no-color master..topic master..unmodified \ >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'simple B...C (unmodified)' ' git range-diff --no-color topic...unmodified >actual && - # same "expected" as above - test_cmp expected actual + # same "expect" as above + test_cmp expect actual ' test_expect_success 'simple A B C (unmodified)' ' git range-diff --no-color master topic unmodified >actual && - # same "expected" as above - test_cmp expected actual + # same "expect" as above + test_cmp expect actual ' test_expect_success 'trivial reordering' ' git range-diff --no-color master topic reordered >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid r1) s/5/A/ 3: $(test_oid t3) = 2: $(test_oid r2) s/11/B/ 4: $(test_oid t4) = 3: $(test_oid r3) s/12/B/ 2: $(test_oid t2) = 4: $(test_oid r4) s/4/A/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'removed a commit' ' git range-diff --no-color master topic removed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid d1) s/5/A/ 2: $(test_oid t2) < -: $(test_oid __) s/4/A/ 3: $(test_oid t3) = 2: $(test_oid d2) s/11/B/ 4: $(test_oid t4) = 3: $(test_oid d3) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'added a commit' ' git range-diff --no-color master topic added >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid a1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid a2) s/4/A/ -: $(test_oid __) > 3: $(test_oid a3) s/6/A/ 3: $(test_oid t3) = 4: $(test_oid a4) s/11/B/ 4: $(test_oid t4) = 5: $(test_oid a5) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'new base, A B C' ' git range-diff --no-color master topic rebased >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid b1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid b2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid b3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid b4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'new base, B...C' ' # this syntax includes the commits from master! git range-diff --no-color topic...rebased >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && -: $(test_oid __) > 1: $(test_oid b5) unrelated 1: $(test_oid t1) = 2: $(test_oid b1) s/5/A/ 2: $(test_oid t2) = 3: $(test_oid b2) s/4/A/ 3: $(test_oid t3) = 4: $(test_oid b3) s/11/B/ 4: $(test_oid t4) = 5: $(test_oid b4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit' ' git range-diff --no-color topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ @@ -226,23 +226,23 @@ test_expect_success 'changed commit' ' +B 13 EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with --no-patch diff option' ' git range-diff --no-color --no-patch topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ 4: $(test_oid t4) ! 4: $(test_oid c4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with --stat diff option' ' git range-diff --no-color --stat topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ a => b | 0 1 file changed, 0 insertions(+), 0 deletions(-) @@ -256,12 +256,12 @@ test_expect_success 'changed commit with --stat diff option' ' a => b | 0 1 file changed, 0 insertions(+), 0 deletions(-) EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with sm config' ' git range-diff --no-color --submodule=log topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ @@ -285,12 +285,12 @@ test_expect_success 'changed commit with sm config' ' +B 13 EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'renamed file' ' git range-diff --no-color --submodule=log topic...renamed-file >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid n1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid n2) s/4/A/ @@ Metadata @@ -330,12 +330,12 @@ test_expect_success 'renamed file' ' Z 10 Z B EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'file with mode only change' ' git range-diff --no-color --submodule=log topic...mode-only-change >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: fccce22 ! 1: 4d39cb3 s/4/A/ @@ Metadata ZAuthor: Thomas Rast <trast@inf.ethz.ch> @@ -370,12 +370,12 @@ test_expect_success 'file with mode only change' ' + ## other-file (mode change 100644 => 100755) ## 3: a63e992 = 3: 4c1e0f5 s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'file added and later removed' ' git range-diff --no-color --submodule=log topic...added-removed >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid s1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid s2) s/4/A/ @@ Metadata @@ -411,7 +411,7 @@ test_expect_success 'file added and later removed' ' + ## new-file (deleted) ## 4: $(test_oid t4) = 4: $(test_oid s4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'no commits on one side' ' @@ -421,7 +421,7 @@ test_expect_success 'no commits on one side' ' test_expect_success 'changed message' ' git range-diff --no-color topic...changed-message >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid m1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid m2) s/4/A/ @@ Metadata @@ -436,7 +436,7 @@ test_expect_success 'changed message' ' 3: $(test_oid t3) = 3: $(test_oid m3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid m4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'dual-coloring' ' -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 5/8] t3206: demonstrate current notes behavior 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (3 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 4/8] t3206: s/expected/expect/ Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-20 4:17 ` Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 6/8] range-diff: output `## Notes ##` header Denton Liu ` (3 subsequent siblings) 8 siblings, 1 reply; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano The test suite had a blindspot where it did not check the behavior of range-diff and format-patch when notes were present. Cover this blindspot. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index e14b4951bb..69babdcf03 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -505,4 +505,56 @@ test_expect_success 'range-diff overrides diff.noprefix internally' ' git -c diff.noprefix=true range-diff HEAD^... ' +test_expect_success 'range-diff compares notes by default' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git range-diff --no-color master..topic master..unmodified \ + >actual && + sed s/Z/\ /g >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ + @@ Metadata + Z + Z ## Commit message ## + Z s/12/B/ + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + test_cmp expect actual +' + +test_expect_success 'format-patch --range-diff compares notes by default' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Metadata + Z + Z ## Commit message ## + Z s/12/B/ + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Metadata/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + test_done -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/8] t3206: demonstrate current notes behavior 2019-11-19 23:55 ` [PATCH v2 5/8] t3206: demonstrate current notes behavior Denton Liu @ 2019-11-20 4:17 ` Junio C Hamano 0 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2019-11-20 4:17 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer Denton Liu <liu.denton@gmail.com> writes: > Subject: Re: [PATCH v2 5/8] t3206: demonstrate current notes behavior This makes it sound as if "current" is either bad or stale and will be "fixed". In the previous round that might have been your intention, but I do not think the focus of v2 is that, so phrasing may want to be updated, perhaps like t3206: range-diff compares log message with commit notes or something. > The test suite had a blindspot where it did not check the behavior of > range-diff and format-patch when notes were present. Cover this > blindspot. Good description. ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 6/8] range-diff: output `## Notes ##` header 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (4 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 5/8] t3206: demonstrate current notes behavior Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` Denton Liu ` (2 subsequent siblings) 8 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano When notes were included in the output of range-diff, they were just mashed together with the rest of the commit message. As a result, users wouldn't be able to clearly distinguish where the commit message ended and where the notes started. Output a `## Notes ##` header when notes are detected so that notes can be compared more clearly. Note that we handle case of `Notes (<ref>): -> ## Notes (<ref>) ##` with this code as well. We can't test this in this patch, however, since there is currently no way to pass along different notes refs to `git log`. This will be fixed in a future patch. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- range-diff.c | 6 ++++++ t/t3206-range-diff.sh | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/range-diff.c b/range-diff.c index 7fed5a3b4b..623397221d 100644 --- a/range-diff.c +++ b/range-diff.c @@ -144,6 +144,12 @@ static int read_patches(const char *range, struct string_list *list) strbuf_addstr(&buf, line); strbuf_addstr(&buf, "\n\n"); strbuf_addstr(&buf, " ## Commit message ##\n"); + } else if (starts_with(line, "Notes") && + line[strlen(line) - 1] == ':') { + strbuf_addstr(&buf, "\n\n"); + /* strip the trailing colon */ + strbuf_addf(&buf, " ## %.*s ##\n", + (int)(strlen(line) - 1), line); } else if (starts_with(line, " ")) { p = line + len - 2; while (isspace(*p) && p >= line) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 69babdcf03..5bb33efdff 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -516,10 +516,10 @@ test_expect_success 'range-diff compares notes by default' ' 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ - @@ Metadata + @@ Commit message Z - Z ## Commit message ## - Z s/12/B/ + Z + Z ## Notes ## - topic note + unmodified note Z @@ -543,17 +543,17 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' grep "= 3: .* s/11/B" 0000-* && grep "! 4: .* s/12/B" 0000-* && sed s/Z/\ /g >expect <<-EOF && - @@ Metadata + @@ Commit message Z - Z ## Commit message ## - Z s/12/B/ + Z + Z ## Notes ## - topic note + unmodified note Z Z ## file ## Z@@ file: A EOF - sed "/@@ Metadata/,/@@ file: A/!d" 0000-* >actual && + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && test_cmp expect actual ' -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (5 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 6/8] range-diff: output `## Notes ##` header Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-20 4:26 ` Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 8/8] format-patch: pass notes configuration to range-diff Denton Liu 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu 8 siblings, 1 reply; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano When a commit being range-diff'd has a note attached to it, the note will be compared as well. However, if a user has multiple notes refs or if they want to suppress notes from being printed, there is currently no way to do this. Passthrough `---no--notes` to the `git log` call so that this option is customizable. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-range-diff.txt | 6 +++- builtin/log.c | 2 +- builtin/range-diff.c | 6 +++- log-tree.c | 2 +- range-diff.c | 15 ++++++---- range-diff.h | 4 ++- t/t3206-range-diff.sh | 47 ++++++++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 10 deletions(-) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index 8a6ea2c6c5..a3d69713f7 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -57,6 +57,10 @@ to revert to color all lines according to the outer diff markers See the ``Algorithm`` section below for an explanation why this is needed. +--[no-]notes[=<treeish>]:: + This flag is passed to the `git log` program + (see linkgit:git-log[1]) that generates the patches. + <range1> <range2>:: Compare the commits specified by the two ranges, where `<range1>` is considered an older version of `<range2>`. @@ -75,7 +79,7 @@ to revert to color all lines according to the outer diff markers linkgit:git-diff[1]), most notably the `--color=[<when>]` and `--no-color` options. These options are used when generating the "diff between patches", i.e. to compare the author, commit message and diff of -corresponding old/new commits. There is currently no means to tweak the +corresponding old/new commits. There is currently no means to tweak most of the diff options passed to `git log` when generating those patches. OUTPUT STABILITY diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..047ac4594d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1189,7 +1189,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); show_range_diff(rev->rdiff1, rev->rdiff2, - rev->creation_factor, 1, &opts); + rev->creation_factor, 1, &opts, NULL); } } diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 9202e75544..98acf3533e 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -15,12 +15,16 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT; struct diff_options diffopt = { NULL }; + struct argv_array other_arg = ARGV_ARRAY_INIT; int simple_color = -1; struct option range_diff_options[] = { OPT_INTEGER(0, "creation-factor", &creation_factor, N_("Percentage by which creation is weighted")), OPT_BOOL(0, "no-dual-color", &simple_color, N_("use simple diff colors")), + OPT_PASSTHRU_ARGV(0, "notes", &other_arg, + N_("notes"), N_("passed to 'git log'"), + PARSE_OPT_OPTARG), OPT_END() }; struct option *options; @@ -78,7 +82,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) FREE_AND_NULL(options); res = show_range_diff(range1.buf, range2.buf, creation_factor, - simple_color < 1, &diffopt); + simple_color < 1, &diffopt, &other_arg); strbuf_release(&range1); strbuf_release(&range2); diff --git a/log-tree.c b/log-tree.c index 923a299e70..151e12f415 100644 --- a/log-tree.c +++ b/log-tree.c @@ -770,7 +770,7 @@ void show_log(struct rev_info *opt) opts.use_color = opt->diffopt.use_color; diff_setup_done(&opts); show_range_diff(opt->rdiff1, opt->rdiff2, - opt->creation_factor, 1, &opts); + opt->creation_factor, 1, &opts, NULL); memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); } diff --git a/range-diff.c b/range-diff.c index 623397221d..f56b4012a2 100644 --- a/range-diff.c +++ b/range-diff.c @@ -40,7 +40,8 @@ static size_t find_end_of_line(char *buffer, unsigned long size) * Reads the patches into a string list, with the `util` field being populated * as struct object_id (will need to be free()d). */ -static int read_patches(const char *range, struct string_list *list) +static int read_patches(const char *range, struct string_list *list, + struct argv_array *other_arg) { struct child_process cp = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT; @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list) "--output-indicator-new=>", "--output-indicator-old=<", "--output-indicator-context=#", - "--no-abbrev-commit", range, + "--no-abbrev-commit", NULL); + if (other_arg) + argv_array_pushv(&cp.args, other_arg->argv); + argv_array_push(&cp.args, range); cp.out = -1; cp.no_stdin = 1; cp.git_cmd = 1; @@ -502,16 +506,17 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data) int show_range_diff(const char *range1, const char *range2, int creation_factor, int dual_color, - struct diff_options *diffopt) + struct diff_options *diffopt, + struct argv_array *other_arg) { int res = 0; struct string_list branch1 = STRING_LIST_INIT_DUP; struct string_list branch2 = STRING_LIST_INIT_DUP; - if (read_patches(range1, &branch1)) + if (read_patches(range1, &branch1, other_arg)) res = error(_("could not parse log for '%s'"), range1); - if (!res && read_patches(range2, &branch2)) + if (!res && read_patches(range2, &branch2, other_arg)) res = error(_("could not parse log for '%s'"), range2); if (!res) { diff --git a/range-diff.h b/range-diff.h index 08a50b6e98..7d918ab9ed 100644 --- a/range-diff.h +++ b/range-diff.h @@ -2,6 +2,7 @@ #define RANGE_DIFF_H #include "diff.h" +#include "argv-array.h" #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 @@ -12,6 +13,7 @@ */ int show_range_diff(const char *range1, const char *range2, int creation_factor, int dual_color, - struct diff_options *diffopt); + struct diff_options *diffopt, + struct argv_array *other_arg); #endif diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 5bb33efdff..cea1846282 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -529,6 +529,53 @@ test_expect_success 'range-diff compares notes by default' ' test_cmp expect actual ' +test_expect_success 'range-diff with --no-notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git range-diff --no-color --no-notes master..topic master..unmodified \ + >actual && + cat >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/ + EOF + test_cmp expect actual +' + +test_expect_success 'range-diff with multiple --notes' ' + git notes --ref=note1 add -m "topic note1" topic && + git notes --ref=note1 add -m "unmodified note1" unmodified && + test_when_finished git notes --ref=note1 remove topic unmodified && + git notes --ref=note2 add -m "topic note2" topic && + git notes --ref=note2 add -m "unmodified note2" unmodified && + test_when_finished git notes --ref=note2 remove topic unmodified && + git range-diff --no-color --notes=note1 --notes=note2 master..topic master..unmodified \ + >actual && + sed s/Z/\ /g >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ + @@ Commit message + Z + Z + Z ## Notes (note1) ## + - topic note1 + + unmodified note1 + Z + Z + Z ## Notes (note2) ## + - topic note2 + + unmodified note2 + Z + Z ## file ## + Z@@ file: A + EOF + test_cmp expect actual +' + test_expect_success 'format-patch --range-diff compares notes by default' ' git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` 2019-11-19 23:55 ` [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` Denton Liu @ 2019-11-20 4:26 ` Junio C Hamano 2019-11-20 19:12 ` Denton Liu 0 siblings, 1 reply; 32+ messages in thread From: Junio C Hamano @ 2019-11-20 4:26 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer Denton Liu <liu.denton@gmail.com> writes: > When a commit being range-diff'd has a note attached to it, the note > will be compared as well. However, if a user has multiple notes refs or > if they want to suppress notes from being printed, there is currently no > way to do this. > > Passthrough `---no--notes` to the `git log` call so that this option is "`--[no-]notes`" or perhaps "`--[no-]notes` and `--notes=<ref>`"? I think the verb phrase is two words, "pass through", by the way. > +--[no-]notes[=<treeish>]:: > + This flag is passed to the `git log` program > + (see linkgit:git-log[1]) that generates the patches. I can see this was taken from "git log --help", and it probably needs fixing for consistency as well, but I think --notes=<ref> would be easier to click users' minds with notes.displayRef configuration variable. > @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list) > "--output-indicator-new=>", > "--output-indicator-old=<", > "--output-indicator-context=#", > - "--no-abbrev-commit", range, > + "--no-abbrev-commit", > NULL); > + if (other_arg) > + argv_array_pushv(&cp.args, other_arg->argv); > + argv_array_push(&cp.args, range); Makes sense. > diff --git a/range-diff.h b/range-diff.h > index 08a50b6e98..7d918ab9ed 100644 > --- a/range-diff.h > +++ b/range-diff.h > @@ -2,6 +2,7 @@ > #define RANGE_DIFF_H > > #include "diff.h" > +#include "argv-array.h" > > #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 > > @@ -12,6 +13,7 @@ > */ > int show_range_diff(const char *range1, const char *range2, > int creation_factor, int dual_color, > - struct diff_options *diffopt); > + struct diff_options *diffopt, > + struct argv_array *other_arg); > > #endif I thought a mere use of "pointer to a struct" did not have to bring the definition of the struct into the picture? In other words, wouldn't it be fine to leave the other_arg a pointer to an opaque structure by not including "argv-array.h" in this file? Thanks. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` 2019-11-20 4:26 ` Junio C Hamano @ 2019-11-20 19:12 ` Denton Liu 2019-11-21 12:43 ` Eric Sunshine 0 siblings, 1 reply; 32+ messages in thread From: Denton Liu @ 2019-11-20 19:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Johannes Schindelin, Thomas Gummerer Hi Junio, On Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > When a commit being range-diff'd has a note attached to it, the note > > will be compared as well. However, if a user has multiple notes refs or > > if they want to suppress notes from being printed, there is currently no > > way to do this. > > > > Passthrough `---no--notes` to the `git log` call so that this option is > > "`--[no-]notes`" or perhaps "`--[no-]notes` and `--notes=<ref>`"? Whoops, I probably typed `ys-` in vim instead of `ys[` so I surrounded the `no-` with the wrong characters. > > I think the verb phrase is two words, "pass through", by the way. Thanks, I didn't know this. > > > +--[no-]notes[=<treeish>]:: > > + This flag is passed to the `git log` program > > + (see linkgit:git-log[1]) that generates the patches. > > I can see this was taken from "git log --help", and it probably > needs fixing for consistency as well, but I think --notes=<ref> > would be easier to click users' minds with notes.displayRef > configuration variable. I'll put in a cleanup patch for this as well. > > > @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list) > > "--output-indicator-new=>", > > "--output-indicator-old=<", > > "--output-indicator-context=#", > > - "--no-abbrev-commit", range, > > + "--no-abbrev-commit", > > NULL); > > + if (other_arg) > > + argv_array_pushv(&cp.args, other_arg->argv); > > + argv_array_push(&cp.args, range); > > Makes sense. > > > diff --git a/range-diff.h b/range-diff.h > > index 08a50b6e98..7d918ab9ed 100644 > > --- a/range-diff.h > > +++ b/range-diff.h > > @@ -2,6 +2,7 @@ > > #define RANGE_DIFF_H > > > > #include "diff.h" > > +#include "argv-array.h" > > > > #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 > > > > @@ -12,6 +13,7 @@ > > */ > > int show_range_diff(const char *range1, const char *range2, > > int creation_factor, int dual_color, > > - struct diff_options *diffopt); > > + struct diff_options *diffopt, > > + struct argv_array *other_arg); > > > > #endif > > I thought a mere use of "pointer to a struct" did not have to bring > the definition of the struct into the picture? In other words, > wouldn't it be fine to leave the other_arg a pointer to an opaque > structure by not including "argv-array.h" in this file? Without including "argv-array.h", we get the following hdr-check error: $ make range-diff.hco HDR range-diff.h In file included from range-diff.hcc:2: ./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility] struct argv_array *other_arg); ^ 1 error generated. make: *** [range-diff.hco] Error 1 I am currently using this compiler for reference: $ clang --version Apple LLVM version 10.0.1 (clang-1001.0.46.4) Target: x86_64-apple-darwin18.7.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin Thanks, Denton > > Thanks. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` 2019-11-20 19:12 ` Denton Liu @ 2019-11-21 12:43 ` Eric Sunshine 2019-11-21 18:35 ` Denton Liu 0 siblings, 1 reply; 32+ messages in thread From: Eric Sunshine @ 2019-11-21 12:43 UTC (permalink / raw) To: Denton Liu Cc: Junio C Hamano, Git Mailing List, Johannes Schindelin, Thomas Gummerer On Wed, Nov 20, 2019 at 2:13 PM Denton Liu <liu.denton@gmail.com> wrote: > On Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote: > > Denton Liu <liu.denton@gmail.com> writes: > > > +#include "argv-array.h" > > > > > > int show_range_diff(const char *range1, const char *range2, > > > int creation_factor, int dual_color, > > > - struct diff_options *diffopt); > > > + struct diff_options *diffopt, > > > + struct argv_array *other_arg); > > > > I thought a mere use of "pointer to a struct" did not have to bring > > the definition of the struct into the picture? In other words, > > wouldn't it be fine to leave the other_arg a pointer to an opaque > > structure by not including "argv-array.h" in this file? > > Without including "argv-array.h", we get the following hdr-check error: > > $ make range-diff.hco > In file included from range-diff.hcc:2: > ./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility] > struct argv_array *other_arg); Did you forward-declare 'argv_array' in range-diff.h? That is, rather than: #include "argv-array.h" instead say: struct argv_array; which tells the compiler that the type exists, thus allowing you to deal with pointers to the struct, as long as you don't try to access any of the struct's members in code which has seen only the forward declaration. You would still need to #include "argv-array.h" in range-diff.c, though. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` 2019-11-21 12:43 ` Eric Sunshine @ 2019-11-21 18:35 ` Denton Liu 0 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-21 18:35 UTC (permalink / raw) To: Eric Sunshine Cc: Junio C Hamano, Git Mailing List, Johannes Schindelin, Thomas Gummerer Hi Eric, On Thu, Nov 21, 2019 at 07:43:18AM -0500, Eric Sunshine wrote: > On Wed, Nov 20, 2019 at 2:13 PM Denton Liu <liu.denton@gmail.com> wrote: > > On Wed, Nov 20, 2019 at 01:26:04PM +0900, Junio C Hamano wrote: > > > Denton Liu <liu.denton@gmail.com> writes: > > > > +#include "argv-array.h" > > > > > > > > int show_range_diff(const char *range1, const char *range2, > > > > int creation_factor, int dual_color, > > > > - struct diff_options *diffopt); > > > > + struct diff_options *diffopt, > > > > + struct argv_array *other_arg); > > > > > > I thought a mere use of "pointer to a struct" did not have to bring > > > the definition of the struct into the picture? In other words, > > > wouldn't it be fine to leave the other_arg a pointer to an opaque > > > structure by not including "argv-array.h" in this file? > > > > Without including "argv-array.h", we get the following hdr-check error: > > > > $ make range-diff.hco > > In file included from range-diff.hcc:2: > > ./range-diff.h:16:14: error: declaration of 'struct argv_array' will not be visible outside of this function [-Werror,-Wvisibility] > > struct argv_array *other_arg); > > Did you forward-declare 'argv_array' in range-diff.h? That is, rather than: > > #include "argv-array.h" > > instead say: > > struct argv_array; *facepalm* Damn, sorry for the brainfart. I completely forgot to do that. That being said, I'd prefer to just include the header for consistency since we already have the #include "diff.h" up there. Or alternatively, I could write a patch to change both into forward-declarations. Personally, I prefer to avoid forward-declarations whenever possible because it adds duplication. If we have a strong preference for one or the other, should we include it in the CodingGuidelines? Thanks, Denton > > which tells the compiler that the type exists, thus allowing you to > deal with pointers to the struct, as long as you don't try to access > any of the struct's members in code which has seen only the forward > declaration. You would still need to #include "argv-array.h" in > range-diff.c, though. ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 8/8] format-patch: pass notes configuration to range-diff 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (6 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` Denton Liu @ 2019-11-19 23:55 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu 8 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-19 23:55 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano Since format-patch accepts `--[no-]notes`, one would expect the range-diff generated to also respect the setting. Unfortunately, the range-diff we currently generate only uses the default option (which always outputs default notes, even when notes are not being used elsewhere). Pass the notes configuration to range-diff so that it can honor it. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- builtin/log.c | 24 +++++++++- t/t3206-range-diff.sh | 101 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 047ac4594d..e192f219d9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1111,6 +1111,25 @@ static void prepare_cover_text(struct pretty_print_context *pp, strbuf_release(&subject_sb); } +static int get_notes_refs(struct string_list_item *item, void *arg) +{ + argv_array_pushf(arg, "--notes=%s", item->string); + return 0; +} + +static void get_notes_args(struct argv_array *arg, struct rev_info *rev) +{ + if (!rev->show_notes) { + argv_array_push(arg, "--no-notes"); + } else if (rev->notes_opt.use_default_notes > 0 || + (rev->notes_opt.use_default_notes == -1 && + !rev->notes_opt.extra_notes_refs.nr)) { + argv_array_push(arg, "--notes"); + } else { + for_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg); + } +} + static void make_cover_letter(struct rev_info *rev, int use_stdout, struct commit *origin, int nr, struct commit **list, @@ -1183,13 +1202,16 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, * can be added later if deemed desirable. */ struct diff_options opts; + struct argv_array other_arg = ARGV_ARRAY_INIT; diff_setup(&opts); opts.file = rev->diffopt.file; opts.use_color = rev->diffopt.use_color; diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); + get_notes_args(&other_arg, rev); show_range_diff(rev->rdiff1, rev->rdiff2, - rev->creation_factor, 1, &opts, NULL); + rev->creation_factor, 1, &opts, &other_arg); + argv_array_clear(&other_arg); } } diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index cea1846282..abbf1efc20 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -576,7 +576,7 @@ test_expect_success 'range-diff with multiple --notes' ' test_cmp expect actual ' -test_expect_success 'format-patch --range-diff compares notes by default' ' +test_expect_success 'format-patch --range-diff does not compare notes by default' ' git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && test_when_finished git notes remove topic unmodified && @@ -588,6 +588,40 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' grep "= 1: .* s/5/A" 0000-* && grep "= 2: .* s/4/A" 0000-* && grep "= 3: .* s/11/B" 0000-* && + grep "= 4: .* s/12/B" 0000-* && + ! grep "Notes" 0000-* && + ! grep "note" 0000-* +' + +test_expect_success 'format-patch --range-diff with --no-notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --no-notes --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "= 4: .* s/12/B" 0000-* && + ! grep "Notes" 0000-* && + ! grep "note" 0000-* +' + +test_expect_success 'format-patch --range-diff with --notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --notes --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && grep "! 4: .* s/12/B" 0000-* && sed s/Z/\ /g >expect <<-EOF && @@ Commit message @@ -604,4 +638,69 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' test_cmp expect actual ' +test_expect_success 'format-patch --range-diff with --notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + test_config format.notes true && + git format-patch --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Commit message + Z + Z + Z ## Notes ## + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + +test_expect_success 'format-patch --range-diff with multiple notes' ' + git notes --ref=note1 add -m "topic note1" topic && + git notes --ref=note1 add -m "unmodified note1" unmodified && + test_when_finished git notes --ref=note1 remove topic unmodified && + git notes --ref=note2 add -m "topic note2" topic && + git notes --ref=note2 add -m "unmodified note2" unmodified && + test_when_finished git notes --ref=note2 remove topic unmodified && + git format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Commit message + Z + Z + Z ## Notes (note1) ## + - topic note1 + + unmodified note1 + Z + Z + Z ## Notes (note2) ## + - topic note2 + + unmodified note2 + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + test_done -- 2.24.0.420.g9ac4901264 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 00/10] range-diff: learn `--notes` 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu ` (7 preceding siblings ...) 2019-11-19 23:55 ` [PATCH v2 8/8] format-patch: pass notes configuration to range-diff Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 01/10] argv-array: add space after `while` Denton Liu ` (9 more replies) 8 siblings, 10 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine This patchset teaches range-diff and format-patch to pass `--notes` options down to the `git log` machinery. This should make the behaviour more configurable for users who either don't want notes to be displayed or want multiple notes refs to be displayed. Changes since v2: * Changed --notes=<treeish> to --notes=<ref> * Added "t3206: disable parameter substitution in heredoc" * Updated the commit message of "range-diff: pass through --notes to `git log`" Changes since v1: * Complete overhaul of approach Denton Liu (10): argv-array: add space after `while` rev-list-options.txt: remove reference to --show-notes pretty-options.txt: --notes accepts a ref instead of treeish t3206: remove spaces after redirect operators t3206: disable parameter substitution in heredoc t3206: s/expected/expect/ t3206: range-diff compares logs with commit notes range-diff: output `## Notes ##` header range-diff: pass through --notes to `git log` format-patch: pass notes configuration to range-diff Documentation/git-range-diff.txt | 6 +- Documentation/pretty-options.txt | 8 +- Documentation/rev-list-options.txt | 2 +- argv-array.c | 2 +- builtin/log.c | 24 ++- builtin/range-diff.c | 6 +- log-tree.c | 2 +- range-diff.c | 21 ++- range-diff.h | 4 +- t/t3206-range-diff.sh | 266 +++++++++++++++++++++++++---- 10 files changed, 291 insertions(+), 50 deletions(-) Range-diff against v2: 1: 396ac06b0d = 1: fd78742570 argv-array: add space after `while` 2: 8cc7c8fe72 = 2: e1b023a6fc rev-list-options.txt: remove reference to --show-notes -: ---------- > 3: 4989956f12 pretty-options.txt: --notes accepts a ref instead of treeish 3: 22f5f07ace = 4: 85fcacf3f9 t3206: remove spaces after redirect operators -: ---------- > 5: 855a3df542 t3206: disable parameter substitution in heredoc 4: 922db36e7e = 6: 92df18b261 t3206: s/expected/expect/ 5: d8ecda2d5e ! 7: 093d32ac4f t3206: demonstrate current notes behavior @@ Metadata Author: Denton Liu <liu.denton@gmail.com> ## Commit message ## - t3206: demonstrate current notes behavior + t3206: range-diff compares logs with commit notes The test suite had a blindspot where it did not check the behavior of range-diff and format-patch when notes were present. Cover this 6: 7dd0b93b0b = 8: 2d1c849ecc range-diff: output `## Notes ##` header 7: 587a02a39c ! 9: 9c144e14c5 range-diff: passthrough --[no-]notes to `git log` @@ Metadata Author: Denton Liu <liu.denton@gmail.com> ## Commit message ## - range-diff: passthrough --[no-]notes to `git log` + range-diff: pass through --notes to `git log` When a commit being range-diff'd has a note attached to it, the note will be compared as well. However, if a user has multiple notes refs or if they want to suppress notes from being printed, there is currently no way to do this. - Passthrough `---no--notes` to the `git log` call so that this option is - customizable. + Pass through `--[no-]notes[=<ref>]` to the `git log` call so that this + option is customizable. ## Documentation/git-range-diff.txt ## @@ Documentation/git-range-diff.txt: to revert to color all lines according to the outer diff markers See the ``Algorithm`` section below for an explanation why this is needed. -+--[no-]notes[=<treeish>]:: ++--[no-]notes[=<ref>]:: + This flag is passed to the `git log` program + (see linkgit:git-log[1]) that generates the patches. + 8: ce8cff7d0c = 10: 86318b3fe7 format-patch: pass notes configuration to range-diff -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 01/10] argv-array: add space after `while` 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 02/10] rev-list-options.txt: remove reference to --show-notes Denton Liu ` (8 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine Signed-off-by: Denton Liu <liu.denton@gmail.com> --- argv-array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/argv-array.c b/argv-array.c index f352ea9357..61ef8c0dfd 100644 --- a/argv-array.c +++ b/argv-array.c @@ -46,7 +46,7 @@ void argv_array_pushl(struct argv_array *array, ...) const char *arg; va_start(ap, array); - while((arg = va_arg(ap, const char *))) + while ((arg = va_arg(ap, const char *))) argv_array_push(array, arg); va_end(ap); } -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 02/10] rev-list-options.txt: remove reference to --show-notes 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu 2019-11-20 21:18 ` [PATCH v3 01/10] argv-array: add space after `while` Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 03/10] pretty-options.txt: --notes accepts a ref instead of treeish Denton Liu ` (7 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine In ab18b2c0df ("log/pretty-options: Document --[no-]notes and deprecate old notes options", 2011-03-30), the `--show-notes` option was deprecated. However, this reference to it still remains. Change it to reference the replacement option: `--notes`. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/rev-list-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 90ff9e2bea..311bc06a9b 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -58,7 +58,7 @@ endif::git-rev-list[] `--all-match`). ifndef::git-rev-list[] + -When `--show-notes` is in effect, the message from the notes is +When `--notes` is in effect, the message from the notes is matched as if it were part of the log message. endif::git-rev-list[] -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 03/10] pretty-options.txt: --notes accepts a ref instead of treeish 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu 2019-11-20 21:18 ` [PATCH v3 01/10] argv-array: add space after `while` Denton Liu 2019-11-20 21:18 ` [PATCH v3 02/10] rev-list-options.txt: remove reference to --show-notes Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 04/10] t3206: remove spaces after redirect operators Denton Liu ` (6 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine Although `--notes=` accepts and handles a tree-ish just fine, it isn't a common use-case for users to pass in bare trees. By having "treeish", it makes it harder to click in users' minds that the argument here is the same type as the `notes.displayRef` configuration variable, for example. Change `treeish` to `ref` so that it will be easier for users to make this connection. Note that we don't completely lose the notion that `--notes=` can accept a tree-ish. In git-notes.txt, we have It is also permitted for a notes ref to point directly to a tree object, in which case the history of the notes can be read with `git log -p -g <refname>`. which means that a hardcore user who wants to take advantage of this obscure use-case will be able to infer the connection and not be completely left in the dark. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/pretty-options.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt index e44fc8f738..6893a4a7ba 100644 --- a/Documentation/pretty-options.txt +++ b/Documentation/pretty-options.txt @@ -57,7 +57,7 @@ message by 4 spaces (i.e. 'medium', which is the default, 'full', and 'fuller'). ifndef::git-rev-list[] ---notes[=<treeish>]:: +--notes[=<ref>]:: Show the notes (see linkgit:git-notes[1]) that annotate the commit, when showing the commit log message. This is the default for `git log`, `git show` and `git whatchanged` commands when @@ -68,8 +68,8 @@ By default, the notes shown are from the notes refs listed in the `core.notesRef` and `notes.displayRef` variables (or corresponding environment overrides). See linkgit:git-config[1] for more details. + -With an optional '<treeish>' argument, use the treeish to find the notes -to display. The treeish can specify the full refname when it begins +With an optional '<ref>' argument, use the ref to find the notes +to display. The ref can specify the full refname when it begins with `refs/notes/`; when it begins with `notes/`, `refs/` and otherwise `refs/notes/` is prefixed to form a full name of the ref. + @@ -85,7 +85,7 @@ being displayed. Examples: "--notes=foo" will show only notes from "--notes --notes=foo --no-notes --notes=bar" will only show notes from "refs/notes/bar". ---show-notes[=<treeish>]:: +--show-notes[=<ref>]:: --[no-]standard-notes:: These options are deprecated. Use the above --notes/--no-notes options instead. -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 04/10] t3206: remove spaces after redirect operators 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (2 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 03/10] pretty-options.txt: --notes accepts a ref instead of treeish Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 05/10] t3206: disable parameter substitution in heredoc Denton Liu ` (5 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine For shell scripts, the usual convention is for there to be no space after redirection operators, (e.g. `>file`, not `> file`). Remove the one instance of this. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 0579cd9969..87c6c029db 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -8,7 +8,7 @@ test_description='range-diff tests' # harm than good. We need some real history. test_expect_success 'setup' ' - git fast-import < "$TEST_DIRECTORY"/t3206/history.export && + git fast-import <"$TEST_DIRECTORY"/t3206/history.export && test_oid_cache <<-EOF # topic t1 sha1:4de457d -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 05/10] t3206: disable parameter substitution in heredoc 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (3 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 04/10] t3206: remove spaces after redirect operators Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 06/10] t3206: s/expected/expect/ Denton Liu ` (4 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine In the first heredoc, parameter substitution is not used so prevent it from happening in the future (perhaps by accident) by escaping the limit EOF. The remaining heredocs use parameter substitution so they cannot be changed. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 87c6c029db..41606775d4 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -9,7 +9,7 @@ test_description='range-diff tests' test_expect_success 'setup' ' git fast-import <"$TEST_DIRECTORY"/t3206/history.export && - test_oid_cache <<-EOF + test_oid_cache <<-\EOF # topic t1 sha1:4de457d t2 sha1:fccce22 -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 06/10] t3206: s/expected/expect/ 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (4 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 05/10] t3206: disable parameter substitution in heredoc Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 07/10] t3206: range-diff compares logs with commit notes Denton Liu ` (3 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine For test cases, the usual convention is to name expected output files "expect", not "expected". Replace all instances of "expected" with "expect". Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 41606775d4..f654aed54f 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -121,88 +121,88 @@ test_expect_success 'setup' ' test_expect_success 'simple A..B A..C (unmodified)' ' git range-diff --no-color master..topic master..unmodified \ >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'simple B...C (unmodified)' ' git range-diff --no-color topic...unmodified >actual && - # same "expected" as above - test_cmp expected actual + # same "expect" as above + test_cmp expect actual ' test_expect_success 'simple A B C (unmodified)' ' git range-diff --no-color master topic unmodified >actual && - # same "expected" as above - test_cmp expected actual + # same "expect" as above + test_cmp expect actual ' test_expect_success 'trivial reordering' ' git range-diff --no-color master topic reordered >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid r1) s/5/A/ 3: $(test_oid t3) = 2: $(test_oid r2) s/11/B/ 4: $(test_oid t4) = 3: $(test_oid r3) s/12/B/ 2: $(test_oid t2) = 4: $(test_oid r4) s/4/A/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'removed a commit' ' git range-diff --no-color master topic removed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid d1) s/5/A/ 2: $(test_oid t2) < -: $(test_oid __) s/4/A/ 3: $(test_oid t3) = 2: $(test_oid d2) s/11/B/ 4: $(test_oid t4) = 3: $(test_oid d3) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'added a commit' ' git range-diff --no-color master topic added >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid a1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid a2) s/4/A/ -: $(test_oid __) > 3: $(test_oid a3) s/6/A/ 3: $(test_oid t3) = 4: $(test_oid a4) s/11/B/ 4: $(test_oid t4) = 5: $(test_oid a5) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'new base, A B C' ' git range-diff --no-color master topic rebased >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid b1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid b2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid b3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid b4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'new base, B...C' ' # this syntax includes the commits from master! git range-diff --no-color topic...rebased >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && -: $(test_oid __) > 1: $(test_oid b5) unrelated 1: $(test_oid t1) = 2: $(test_oid b1) s/5/A/ 2: $(test_oid t2) = 3: $(test_oid b2) s/4/A/ 3: $(test_oid t3) = 4: $(test_oid b3) s/11/B/ 4: $(test_oid t4) = 5: $(test_oid b4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit' ' git range-diff --no-color topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ @@ -226,23 +226,23 @@ test_expect_success 'changed commit' ' +B 13 EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with --no-patch diff option' ' git range-diff --no-color --no-patch topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ 4: $(test_oid t4) ! 4: $(test_oid c4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with --stat diff option' ' git range-diff --no-color --stat topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ a => b | 0 1 file changed, 0 insertions(+), 0 deletions(-) @@ -256,12 +256,12 @@ test_expect_success 'changed commit with --stat diff option' ' a => b | 0 1 file changed, 0 insertions(+), 0 deletions(-) EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'changed commit with sm config' ' git range-diff --no-color --submodule=log topic...changed >actual && - cat >expected <<-EOF && + cat >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/ 2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/ 3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/ @@ -285,12 +285,12 @@ test_expect_success 'changed commit with sm config' ' +B 13 EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'renamed file' ' git range-diff --no-color --submodule=log topic...renamed-file >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid n1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid n2) s/4/A/ @@ Metadata @@ -330,12 +330,12 @@ test_expect_success 'renamed file' ' Z 10 Z B EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'file with mode only change' ' git range-diff --no-color --submodule=log topic...mode-only-change >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: fccce22 ! 1: 4d39cb3 s/4/A/ @@ Metadata ZAuthor: Thomas Rast <trast@inf.ethz.ch> @@ -370,12 +370,12 @@ test_expect_success 'file with mode only change' ' + ## other-file (mode change 100644 => 100755) ## 3: a63e992 = 3: 4c1e0f5 s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'file added and later removed' ' git range-diff --no-color --submodule=log topic...added-removed >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid s1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid s2) s/4/A/ @@ Metadata @@ -411,7 +411,7 @@ test_expect_success 'file added and later removed' ' + ## new-file (deleted) ## 4: $(test_oid t4) = 4: $(test_oid s4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'no commits on one side' ' @@ -421,7 +421,7 @@ test_expect_success 'no commits on one side' ' test_expect_success 'changed message' ' git range-diff --no-color topic...changed-message >actual && - sed s/Z/\ /g >expected <<-EOF && + sed s/Z/\ /g >expect <<-EOF && 1: $(test_oid t1) = 1: $(test_oid m1) s/5/A/ 2: $(test_oid t2) ! 2: $(test_oid m2) s/4/A/ @@ Metadata @@ -436,7 +436,7 @@ test_expect_success 'changed message' ' 3: $(test_oid t3) = 3: $(test_oid m3) s/11/B/ 4: $(test_oid t4) = 4: $(test_oid m4) s/12/B/ EOF - test_cmp expected actual + test_cmp expect actual ' test_expect_success 'dual-coloring' ' -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 07/10] t3206: range-diff compares logs with commit notes 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (5 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 06/10] t3206: s/expected/expect/ Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 08/10] range-diff: output `## Notes ##` header Denton Liu ` (2 subsequent siblings) 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine The test suite had a blindspot where it did not check the behavior of range-diff and format-patch when notes were present. Cover this blindspot. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3206-range-diff.sh | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index f654aed54f..19ba644933 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -505,4 +505,56 @@ test_expect_success 'range-diff overrides diff.noprefix internally' ' git -c diff.noprefix=true range-diff HEAD^... ' +test_expect_success 'range-diff compares notes by default' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git range-diff --no-color master..topic master..unmodified \ + >actual && + sed s/Z/\ /g >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ + @@ Metadata + Z + Z ## Commit message ## + Z s/12/B/ + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + test_cmp expect actual +' + +test_expect_success 'format-patch --range-diff compares notes by default' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Metadata + Z + Z ## Commit message ## + Z s/12/B/ + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Metadata/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + test_done -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 08/10] range-diff: output `## Notes ##` header 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (6 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 07/10] t3206: range-diff compares logs with commit notes Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 09/10] range-diff: pass through --notes to `git log` Denton Liu 2019-11-20 21:18 ` [PATCH v3 10/10] format-patch: pass notes configuration to range-diff Denton Liu 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine When notes were included in the output of range-diff, they were just mashed together with the rest of the commit message. As a result, users wouldn't be able to clearly distinguish where the commit message ended and where the notes started. Output a `## Notes ##` header when notes are detected so that notes can be compared more clearly. Note that we handle case of `Notes (<ref>): -> ## Notes (<ref>) ##` with this code as well. We can't test this in this patch, however, since there is currently no way to pass along different notes refs to `git log`. This will be fixed in a future patch. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- range-diff.c | 6 ++++++ t/t3206-range-diff.sh | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/range-diff.c b/range-diff.c index 7fed5a3b4b..623397221d 100644 --- a/range-diff.c +++ b/range-diff.c @@ -144,6 +144,12 @@ static int read_patches(const char *range, struct string_list *list) strbuf_addstr(&buf, line); strbuf_addstr(&buf, "\n\n"); strbuf_addstr(&buf, " ## Commit message ##\n"); + } else if (starts_with(line, "Notes") && + line[strlen(line) - 1] == ':') { + strbuf_addstr(&buf, "\n\n"); + /* strip the trailing colon */ + strbuf_addf(&buf, " ## %.*s ##\n", + (int)(strlen(line) - 1), line); } else if (starts_with(line, " ")) { p = line + len - 2; while (isspace(*p) && p >= line) diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 19ba644933..b936c16dd1 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -516,10 +516,10 @@ test_expect_success 'range-diff compares notes by default' ' 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ - @@ Metadata + @@ Commit message Z - Z ## Commit message ## - Z s/12/B/ + Z + Z ## Notes ## - topic note + unmodified note Z @@ -543,17 +543,17 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' grep "= 3: .* s/11/B" 0000-* && grep "! 4: .* s/12/B" 0000-* && sed s/Z/\ /g >expect <<-EOF && - @@ Metadata + @@ Commit message Z - Z ## Commit message ## - Z s/12/B/ + Z + Z ## Notes ## - topic note + unmodified note Z Z ## file ## Z@@ file: A EOF - sed "/@@ Metadata/,/@@ file: A/!d" 0000-* >actual && + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && test_cmp expect actual ' -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 09/10] range-diff: pass through --notes to `git log` 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (7 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 08/10] range-diff: output `## Notes ##` header Denton Liu @ 2019-11-20 21:18 ` Denton Liu 2019-11-20 21:18 ` [PATCH v3 10/10] format-patch: pass notes configuration to range-diff Denton Liu 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine When a commit being range-diff'd has a note attached to it, the note will be compared as well. However, if a user has multiple notes refs or if they want to suppress notes from being printed, there is currently no way to do this. Pass through `--[no-]notes[=<ref>]` to the `git log` call so that this option is customizable. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-range-diff.txt | 6 +++- builtin/log.c | 2 +- builtin/range-diff.c | 6 +++- log-tree.c | 2 +- range-diff.c | 15 ++++++---- range-diff.h | 4 ++- t/t3206-range-diff.sh | 47 ++++++++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 10 deletions(-) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index 8a6ea2c6c5..a2c25f4490 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -57,6 +57,10 @@ to revert to color all lines according to the outer diff markers See the ``Algorithm`` section below for an explanation why this is needed. +--[no-]notes[=<ref>]:: + This flag is passed to the `git log` program + (see linkgit:git-log[1]) that generates the patches. + <range1> <range2>:: Compare the commits specified by the two ranges, where `<range1>` is considered an older version of `<range2>`. @@ -75,7 +79,7 @@ to revert to color all lines according to the outer diff markers linkgit:git-diff[1]), most notably the `--color=[<when>]` and `--no-color` options. These options are used when generating the "diff between patches", i.e. to compare the author, commit message and diff of -corresponding old/new commits. There is currently no means to tweak the +corresponding old/new commits. There is currently no means to tweak most of the diff options passed to `git log` when generating those patches. OUTPUT STABILITY diff --git a/builtin/log.c b/builtin/log.c index a26f223ab4..047ac4594d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1189,7 +1189,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); show_range_diff(rev->rdiff1, rev->rdiff2, - rev->creation_factor, 1, &opts); + rev->creation_factor, 1, &opts, NULL); } } diff --git a/builtin/range-diff.c b/builtin/range-diff.c index 9202e75544..98acf3533e 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -15,12 +15,16 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) { int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT; struct diff_options diffopt = { NULL }; + struct argv_array other_arg = ARGV_ARRAY_INIT; int simple_color = -1; struct option range_diff_options[] = { OPT_INTEGER(0, "creation-factor", &creation_factor, N_("Percentage by which creation is weighted")), OPT_BOOL(0, "no-dual-color", &simple_color, N_("use simple diff colors")), + OPT_PASSTHRU_ARGV(0, "notes", &other_arg, + N_("notes"), N_("passed to 'git log'"), + PARSE_OPT_OPTARG), OPT_END() }; struct option *options; @@ -78,7 +82,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) FREE_AND_NULL(options); res = show_range_diff(range1.buf, range2.buf, creation_factor, - simple_color < 1, &diffopt); + simple_color < 1, &diffopt, &other_arg); strbuf_release(&range1); strbuf_release(&range2); diff --git a/log-tree.c b/log-tree.c index 923a299e70..151e12f415 100644 --- a/log-tree.c +++ b/log-tree.c @@ -770,7 +770,7 @@ void show_log(struct rev_info *opt) opts.use_color = opt->diffopt.use_color; diff_setup_done(&opts); show_range_diff(opt->rdiff1, opt->rdiff2, - opt->creation_factor, 1, &opts); + opt->creation_factor, 1, &opts, NULL); memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); } diff --git a/range-diff.c b/range-diff.c index 623397221d..f56b4012a2 100644 --- a/range-diff.c +++ b/range-diff.c @@ -40,7 +40,8 @@ static size_t find_end_of_line(char *buffer, unsigned long size) * Reads the patches into a string list, with the `util` field being populated * as struct object_id (will need to be free()d). */ -static int read_patches(const char *range, struct string_list *list) +static int read_patches(const char *range, struct string_list *list, + struct argv_array *other_arg) { struct child_process cp = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT, contents = STRBUF_INIT; @@ -61,8 +62,11 @@ static int read_patches(const char *range, struct string_list *list) "--output-indicator-new=>", "--output-indicator-old=<", "--output-indicator-context=#", - "--no-abbrev-commit", range, + "--no-abbrev-commit", NULL); + if (other_arg) + argv_array_pushv(&cp.args, other_arg->argv); + argv_array_push(&cp.args, range); cp.out = -1; cp.no_stdin = 1; cp.git_cmd = 1; @@ -502,16 +506,17 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data) int show_range_diff(const char *range1, const char *range2, int creation_factor, int dual_color, - struct diff_options *diffopt) + struct diff_options *diffopt, + struct argv_array *other_arg) { int res = 0; struct string_list branch1 = STRING_LIST_INIT_DUP; struct string_list branch2 = STRING_LIST_INIT_DUP; - if (read_patches(range1, &branch1)) + if (read_patches(range1, &branch1, other_arg)) res = error(_("could not parse log for '%s'"), range1); - if (!res && read_patches(range2, &branch2)) + if (!res && read_patches(range2, &branch2, other_arg)) res = error(_("could not parse log for '%s'"), range2); if (!res) { diff --git a/range-diff.h b/range-diff.h index 08a50b6e98..7d918ab9ed 100644 --- a/range-diff.h +++ b/range-diff.h @@ -2,6 +2,7 @@ #define RANGE_DIFF_H #include "diff.h" +#include "argv-array.h" #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60 @@ -12,6 +13,7 @@ */ int show_range_diff(const char *range1, const char *range2, int creation_factor, int dual_color, - struct diff_options *diffopt); + struct diff_options *diffopt, + struct argv_array *other_arg); #endif diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index b936c16dd1..521b4a83ec 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -529,6 +529,53 @@ test_expect_success 'range-diff compares notes by default' ' test_cmp expect actual ' +test_expect_success 'range-diff with --no-notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git range-diff --no-color --no-notes master..topic master..unmodified \ + >actual && + cat >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/ + EOF + test_cmp expect actual +' + +test_expect_success 'range-diff with multiple --notes' ' + git notes --ref=note1 add -m "topic note1" topic && + git notes --ref=note1 add -m "unmodified note1" unmodified && + test_when_finished git notes --ref=note1 remove topic unmodified && + git notes --ref=note2 add -m "topic note2" topic && + git notes --ref=note2 add -m "unmodified note2" unmodified && + test_when_finished git notes --ref=note2 remove topic unmodified && + git range-diff --no-color --notes=note1 --notes=note2 master..topic master..unmodified \ + >actual && + sed s/Z/\ /g >expect <<-EOF && + 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/ + 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/ + 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/ + 4: $(test_oid t4) ! 4: $(test_oid u4) s/12/B/ + @@ Commit message + Z + Z + Z ## Notes (note1) ## + - topic note1 + + unmodified note1 + Z + Z + Z ## Notes (note2) ## + - topic note2 + + unmodified note2 + Z + Z ## file ## + Z@@ file: A + EOF + test_cmp expect actual +' + test_expect_success 'format-patch --range-diff compares notes by default' ' git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 10/10] format-patch: pass notes configuration to range-diff 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu ` (8 preceding siblings ...) 2019-11-20 21:18 ` [PATCH v3 09/10] range-diff: pass through --notes to `git log` Denton Liu @ 2019-11-20 21:18 ` Denton Liu 9 siblings, 0 replies; 32+ messages in thread From: Denton Liu @ 2019-11-20 21:18 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, Thomas Gummerer, Junio C Hamano, Eric Sunshine Since format-patch accepts `--[no-]notes`, one would expect the range-diff generated to also respect the setting. Unfortunately, the range-diff we currently generate only uses the default option (which always outputs default notes, even when notes are not being used elsewhere). Pass the notes configuration to range-diff so that it can honor it. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- builtin/log.c | 24 +++++++++- t/t3206-range-diff.sh | 101 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 047ac4594d..e192f219d9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1111,6 +1111,25 @@ static void prepare_cover_text(struct pretty_print_context *pp, strbuf_release(&subject_sb); } +static int get_notes_refs(struct string_list_item *item, void *arg) +{ + argv_array_pushf(arg, "--notes=%s", item->string); + return 0; +} + +static void get_notes_args(struct argv_array *arg, struct rev_info *rev) +{ + if (!rev->show_notes) { + argv_array_push(arg, "--no-notes"); + } else if (rev->notes_opt.use_default_notes > 0 || + (rev->notes_opt.use_default_notes == -1 && + !rev->notes_opt.extra_notes_refs.nr)) { + argv_array_push(arg, "--notes"); + } else { + for_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg); + } +} + static void make_cover_letter(struct rev_info *rev, int use_stdout, struct commit *origin, int nr, struct commit **list, @@ -1183,13 +1202,16 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, * can be added later if deemed desirable. */ struct diff_options opts; + struct argv_array other_arg = ARGV_ARRAY_INIT; diff_setup(&opts); opts.file = rev->diffopt.file; opts.use_color = rev->diffopt.use_color; diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); + get_notes_args(&other_arg, rev); show_range_diff(rev->rdiff1, rev->rdiff2, - rev->creation_factor, 1, &opts, NULL); + rev->creation_factor, 1, &opts, &other_arg); + argv_array_clear(&other_arg); } } diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index 521b4a83ec..ec2b456dbb 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -576,7 +576,7 @@ test_expect_success 'range-diff with multiple --notes' ' test_cmp expect actual ' -test_expect_success 'format-patch --range-diff compares notes by default' ' +test_expect_success 'format-patch --range-diff does not compare notes by default' ' git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && test_when_finished git notes remove topic unmodified && @@ -588,6 +588,40 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' grep "= 1: .* s/5/A" 0000-* && grep "= 2: .* s/4/A" 0000-* && grep "= 3: .* s/11/B" 0000-* && + grep "= 4: .* s/12/B" 0000-* && + ! grep "Notes" 0000-* && + ! grep "note" 0000-* +' + +test_expect_success 'format-patch --range-diff with --no-notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --no-notes --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "= 4: .* s/12/B" 0000-* && + ! grep "Notes" 0000-* && + ! grep "note" 0000-* +' + +test_expect_success 'format-patch --range-diff with --notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + git format-patch --notes --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && grep "! 4: .* s/12/B" 0000-* && sed s/Z/\ /g >expect <<-EOF && @@ Commit message @@ -604,4 +638,69 @@ test_expect_success 'format-patch --range-diff compares notes by default' ' test_cmp expect actual ' +test_expect_success 'format-patch --range-diff with --notes' ' + git notes add -m "topic note" topic && + git notes add -m "unmodified note" unmodified && + test_when_finished git notes remove topic unmodified && + test_config format.notes true && + git format-patch --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Commit message + Z + Z + Z ## Notes ## + - topic note + + unmodified note + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + +test_expect_success 'format-patch --range-diff with multiple notes' ' + git notes --ref=note1 add -m "topic note1" topic && + git notes --ref=note1 add -m "unmodified note1" unmodified && + test_when_finished git notes --ref=note1 remove topic unmodified && + git notes --ref=note2 add -m "topic note2" topic && + git notes --ref=note2 add -m "unmodified note2" unmodified && + test_when_finished git notes --ref=note2 remove topic unmodified && + git format-patch --notes=note1 --notes=note2 --cover-letter --range-diff=$prev \ + master..unmodified >actual && + test_when_finished "rm 000?-*" && + test_line_count = 5 actual && + test_i18ngrep "^Range-diff:$" 0000-* && + grep "= 1: .* s/5/A" 0000-* && + grep "= 2: .* s/4/A" 0000-* && + grep "= 3: .* s/11/B" 0000-* && + grep "! 4: .* s/12/B" 0000-* && + sed s/Z/\ /g >expect <<-EOF && + @@ Commit message + Z + Z + Z ## Notes (note1) ## + - topic note1 + + unmodified note1 + Z + Z + Z ## Notes (note2) ## + - topic note2 + + unmodified note2 + Z + Z ## file ## + Z@@ file: A + EOF + sed "/@@ Commit message/,/@@ file: A/!d" 0000-* >actual && + test_cmp expect actual +' + test_done -- 2.24.0.450.g7a9a4598a9 ^ permalink raw reply related [flat|nested] 32+ messages in thread
end of thread, other threads:[~2019-11-21 18:35 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-19 1:06 [PATCH 0/3] range-diff: don't compare notes Denton Liu 2019-11-19 1:06 ` [PATCH 1/3] t3206: remove spaces after redirect operators Denton Liu 2019-11-19 1:06 ` [PATCH 2/3] t3206: demonstrate failure with notes Denton Liu 2019-11-19 3:03 ` Junio C Hamano 2019-11-19 1:06 ` [PATCH 3/3] range-diff: use --no-notes to generate patches Denton Liu 2019-11-19 2:56 ` [PATCH 0/3] range-diff: don't compare notes Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 0/8] range-diff: learn `--notes` Denton Liu 2019-11-19 23:55 ` [PATCH v2 1/8] argv-array: add space after `while` Denton Liu 2019-11-19 23:55 ` [PATCH v2 2/8] rev-list-options.txt: remove reference to --show-notes Denton Liu 2019-11-19 23:55 ` [PATCH v2 3/8] t3206: remove spaces after redirect operators Denton Liu 2019-11-20 0:20 ` Eric Sunshine 2019-11-19 23:55 ` [PATCH v2 4/8] t3206: s/expected/expect/ Denton Liu 2019-11-19 23:55 ` [PATCH v2 5/8] t3206: demonstrate current notes behavior Denton Liu 2019-11-20 4:17 ` Junio C Hamano 2019-11-19 23:55 ` [PATCH v2 6/8] range-diff: output `## Notes ##` header Denton Liu 2019-11-19 23:55 ` [PATCH v2 7/8] range-diff: passthrough --[no-]notes to `git log` Denton Liu 2019-11-20 4:26 ` Junio C Hamano 2019-11-20 19:12 ` Denton Liu 2019-11-21 12:43 ` Eric Sunshine 2019-11-21 18:35 ` Denton Liu 2019-11-19 23:55 ` [PATCH v2 8/8] format-patch: pass notes configuration to range-diff Denton Liu 2019-11-20 21:18 ` [PATCH v3 00/10] range-diff: learn `--notes` Denton Liu 2019-11-20 21:18 ` [PATCH v3 01/10] argv-array: add space after `while` Denton Liu 2019-11-20 21:18 ` [PATCH v3 02/10] rev-list-options.txt: remove reference to --show-notes Denton Liu 2019-11-20 21:18 ` [PATCH v3 03/10] pretty-options.txt: --notes accepts a ref instead of treeish Denton Liu 2019-11-20 21:18 ` [PATCH v3 04/10] t3206: remove spaces after redirect operators Denton Liu 2019-11-20 21:18 ` [PATCH v3 05/10] t3206: disable parameter substitution in heredoc Denton Liu 2019-11-20 21:18 ` [PATCH v3 06/10] t3206: s/expected/expect/ Denton Liu 2019-11-20 21:18 ` [PATCH v3 07/10] t3206: range-diff compares logs with commit notes Denton Liu 2019-11-20 21:18 ` [PATCH v3 08/10] range-diff: output `## Notes ##` header Denton Liu 2019-11-20 21:18 ` [PATCH v3 09/10] range-diff: pass through --notes to `git log` Denton Liu 2019-11-20 21:18 ` [PATCH v3 10/10] format-patch: pass notes configuration to range-diff Denton Liu
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).