| Message ID | 10b11526206d3b515ba08ac80ccf09ecb7a03420.1637056178.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | diff --color-moved[-ws] speedups | expand |
Hi Phillip, The commit's oneline has a typo: zerba instead of zebra. On Tue, 16 Nov 2021, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When marking moved lines it is possible for a block of potential > matched lines to extend past a change in sign when there is a sequence > of added lines whose text matches the text of a sequence of deleted > and added lines. Most of the time either `match` will be NULL or > `pmb_advance_or_null()` will fail when the loop encounters a change of > sign but there are corner cases where `match` is non-NULL and > `pmb_advance_or_null()` successfully advances the moved block despite > the change in sign. > > One consequence of this is highlighting a short line as moved when it > should not be. For example > > -moved line # Correctly highlighted as moved > +short line # Wrongly highlighted as moved > context > +moved line # Correctly highlighted as moved > +short line > context > -short line > > The other consequence is coloring a moved addition following a moved > deletion in the wrong color. In the example below the first "+moved > line 3" should be highlighted as newMoved not newMovedAlternate. > > -moved line 1 # Correctly highlighted as oldMoved > -moved line 2 # Correctly highlighted as oldMovedAlternate > +moved line 3 # Wrongly highlighted as newMovedAlternate > context # Everything else is highlighted correctly > +moved line 2 > +moved line 3 > context > +moved line 1 > -moved line 3 > > These false matches are more likely when using --color-moved-ws with > the exception of --color-moved-ws=allow-indentation-change which ties > the sign of the current whitespace delta to the sign of the line to > avoid this problem. The fix is to check that the sign of the new line > being matched is the same as the sign of the line that started the > block of potential matches. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > diff.c | 17 ++++++---- > t/t4015-diff-whitespace.sh | 65 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 76 insertions(+), 6 deletions(-) > > diff --git a/diff.c b/diff.c > index 53f0df75329..efba2789354 100644 > --- a/diff.c > +++ b/diff.c > @@ -1176,7 +1176,7 @@ static void mark_color_as_moved(struct diff_options *o, > struct moved_block *pmb = NULL; /* potentially moved blocks */ > int pmb_nr = 0, pmb_alloc = 0; > int n, flipped_block = 0, block_length = 0; > - enum diff_symbol last_symbol = 0; > + enum diff_symbol moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; The exact value does not matter, as long as it is different from whatever the next line will have, of course. > > > for (n = 0; n < o->emitted_symbols->nr; n++) { > @@ -1202,7 +1202,7 @@ static void mark_color_as_moved(struct diff_options *o, > flipped_block = 0; > } > > - if (!match) { > + if (pmb_nr && (!match || l->s != moved_symbol)) { > int i; > > if (!adjust_last_block(o, n, block_length) && > @@ -1219,12 +1219,13 @@ static void mark_color_as_moved(struct diff_options *o, > pmb_nr = 0; > block_length = 0; > flipped_block = 0; > - last_symbol = l->s; > + } This is one of those instances where I dislike having the patch in a static mail. I so want to have a _convenient_ way to expand the diff context, to have a look around. So I went over to https://github.com/gitgitgadget/git/pull/981/commits/10b11526206d3b515ba08ac80ccf09ecb7a03420 to get the convenience I need for a pleasant reviewing experience. In this instance, the `continue` that dropped out of that conditional block gave me pause. My understanding is that the diff makes it essentially a lot harder to understand what is done here: this conditional block did two things, it re-set the possibly-moved-block, and it skipped to the next loop iteration. With this patch, we now re-set the possibly-moved-block in more cases, but still skip to the next loop iteration under the same condition as before: > + if (!match) { > + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; > continue; > } However, after reading the commit message, I would have expected the condition above to read `if (!match || l->s != moved_symbol)` instead of `if (!match)`. Could you help me understand what I am missing? > > if (o->color_moved == COLOR_MOVED_PLAIN) { > - last_symbol = l->s; > l->flags |= DIFF_SYMBOL_MOVED_LINE; > continue; > } I want to make sure that I understand why the `last_symbol` assignment could be removed without any `moved_symbol` assignment in its place. But I don't, I still do not see why we do not need a `moved_symbol = l->s;` assignment here. Unless, that is, we extended the `!match` condition above to also cover the case where `l->s != moved_symbol`. > @@ -1251,11 +1252,16 @@ static void mark_color_as_moved(struct diff_options *o, > &pmb, &pmb_alloc, > &pmb_nr); > > - if (contiguous && pmb_nr && last_symbol == l->s) > + if (contiguous && pmb_nr && moved_symbol == l->s) > flipped_block = (flipped_block + 1) % 2; This is totally not your fault, but I really wish we could have the much simpler and much easier to understand `flipped_block = !flipped_block` here. > else > flipped_block = 0; > > + if (pmb_nr) > + moved_symbol = l->s; > + else > + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; > + > block_length = 0; > } > > @@ -1265,7 +1271,6 @@ static void mark_color_as_moved(struct diff_options *o, > if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS) > l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT; > } > - last_symbol = l->s; That makes sense: we only set `moved_symbol` when `pmb_nr` had been 0 now, and don't want it to be overridden. As I said, I do not quite understand this patch yet, and am looking for your guidance to wrap my head around it. Thank you for working on this! Dscho > } > adjust_last_block(o, n, block_length); > > diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh > index 4e0fd76c6c5..15782c879d2 100755 > --- a/t/t4015-diff-whitespace.sh > +++ b/t/t4015-diff-whitespace.sh > @@ -1514,6 +1514,71 @@ test_expect_success 'zebra alternate color is only used when necessary' ' > test_cmp expected actual > ' > > +test_expect_success 'short lines of opposite sign do not get marked as moved' ' > + cat >old.txt <<-\EOF && > + this line should be marked as moved > + unchanged > + unchanged > + unchanged > + unchanged > + too short > + this line should be marked as oldMoved newMoved > + this line should be marked as oldMovedAlternate newMoved > + unchanged 1 > + unchanged 2 > + unchanged 3 > + unchanged 4 > + this line should be marked as oldMoved newMoved/newMovedAlternate > + EOF > + cat >new.txt <<-\EOF && > + too short > + unchanged > + unchanged > + this line should be marked as moved > + too short > + unchanged > + unchanged > + this line should be marked as oldMoved newMoved/newMovedAlternate > + unchanged 1 > + unchanged 2 > + this line should be marked as oldMovedAlternate newMoved > + this line should be marked as oldMoved newMoved/newMovedAlternate > + unchanged 3 > + this line should be marked as oldMoved newMoved > + unchanged 4 > + EOF > + test_expect_code 1 git diff --no-index --color --color-moved=zebra \ > + old.txt new.txt >output && cat output && > + grep -v index output | test_decode_color >actual && > + cat >expect <<-\EOF && > + <BOLD>diff --git a/old.txt b/new.txt<RESET> > + <BOLD>--- a/old.txt<RESET> > + <BOLD>+++ b/new.txt<RESET> > + <CYAN>@@ -1,13 +1,15 @@<RESET> > + <BOLD;MAGENTA>-this line should be marked as moved<RESET> > + <GREEN>+<RESET><GREEN>too short<RESET> > + unchanged<RESET> > + unchanged<RESET> > + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as moved<RESET> > + <GREEN>+<RESET><GREEN>too short<RESET> > + unchanged<RESET> > + unchanged<RESET> > + <RED>-too short<RESET> > + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved<RESET> > + <BOLD;BLUE>-this line should be marked as oldMovedAlternate newMoved<RESET> > + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> > + unchanged 1<RESET> > + unchanged 2<RESET> > + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMovedAlternate newMoved<RESET> > + <BOLD;YELLOW>+<RESET><BOLD;YELLOW>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> > + unchanged 3<RESET> > + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved<RESET> > + unchanged 4<RESET> > + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> > + EOF > + test_cmp expect actual > +' > + > test_expect_success 'cmd option assumes configured colored-moved' ' > test_config color.diff.oldMoved "magenta" && > test_config color.diff.newMoved "cyan" && > -- > gitgitgadget > >
Hi Dscho Thanks ever so much for taking a detailed look at this series. On 22/11/2021 14:18, Johannes Schindelin wrote: > Hi Phillip, > > The commit's oneline has a typo: zerba instead of zebra. Sigh, I thought I'd fixed that > On Tue, 16 Nov 2021, Phillip Wood via GitGitGadget wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> When marking moved lines it is possible for a block of potential >> matched lines to extend past a change in sign when there is a sequence >> of added lines whose text matches the text of a sequence of deleted >> and added lines. Most of the time either `match` will be NULL or >> `pmb_advance_or_null()` will fail when the loop encounters a change of >> sign but there are corner cases where `match` is non-NULL and >> `pmb_advance_or_null()` successfully advances the moved block despite >> the change in sign. >> >> One consequence of this is highlighting a short line as moved when it >> should not be. For example >> >> -moved line # Correctly highlighted as moved >> +short line # Wrongly highlighted as moved >> context >> +moved line # Correctly highlighted as moved >> +short line >> context >> -short line >> >> The other consequence is coloring a moved addition following a moved >> deletion in the wrong color. In the example below the first "+moved >> line 3" should be highlighted as newMoved not newMovedAlternate. >> >> -moved line 1 # Correctly highlighted as oldMoved >> -moved line 2 # Correctly highlighted as oldMovedAlternate >> +moved line 3 # Wrongly highlighted as newMovedAlternate >> context # Everything else is highlighted correctly >> +moved line 2 >> +moved line 3 >> context >> +moved line 1 >> -moved line 3 >> >> These false matches are more likely when using --color-moved-ws with >> the exception of --color-moved-ws=allow-indentation-change which ties >> the sign of the current whitespace delta to the sign of the line to >> avoid this problem. The fix is to check that the sign of the new line >> being matched is the same as the sign of the line that started the >> block of potential matches. >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> diff.c | 17 ++++++---- >> t/t4015-diff-whitespace.sh | 65 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 76 insertions(+), 6 deletions(-) >> >> diff --git a/diff.c b/diff.c >> index 53f0df75329..efba2789354 100644 >> --- a/diff.c >> +++ b/diff.c >> @@ -1176,7 +1176,7 @@ static void mark_color_as_moved(struct diff_options *o, >> struct moved_block *pmb = NULL; /* potentially moved blocks */ >> int pmb_nr = 0, pmb_alloc = 0; >> int n, flipped_block = 0, block_length = 0; >> - enum diff_symbol last_symbol = 0; >> + enum diff_symbol moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; > > The exact value does not matter, as long as it is different from whatever > the next line will have, of course. > >> >> >> for (n = 0; n < o->emitted_symbols->nr; n++) { >> @@ -1202,7 +1202,7 @@ static void mark_color_as_moved(struct diff_options *o, >> flipped_block = 0; >> } >> >> - if (!match) { >> + if (pmb_nr && (!match || l->s != moved_symbol)) { >> int i; >> >> if (!adjust_last_block(o, n, block_length) && >> @@ -1219,12 +1219,13 @@ static void mark_color_as_moved(struct diff_options *o, >> pmb_nr = 0; >> block_length = 0; >> flipped_block = 0; >> - last_symbol = l->s; >> + } > > This is one of those instances where I dislike having the patch in a > static mail. I so want to have a _convenient_ way to expand the diff > context, to have a look around. > > So I went over to > https://github.com/gitgitgadget/git/pull/981/commits/10b11526206d3b515ba08ac80ccf09ecb7a03420 > to get the convenience I need for a pleasant reviewing experience. > > In this instance, the `continue` that dropped out of that conditional > block gave me pause. > > My understanding is that the diff makes it essentially a lot harder to > understand what is done here: this conditional block did two things, it > re-set the possibly-moved-block, and it skipped to the next loop > iteration. With this patch, we now re-set the possibly-moved-block in more > cases, but still skip to the next loop iteration under the same condition > as before: > >> + if (!match) { >> + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; >> continue; >> } > > However, after reading the commit message, I would have expected the > condition above to read `if (!match || l->s != moved_symbol)` instead of > `if (!match)`. Could you help me understand what I am missing? If there is a match we want to carry on executing the body of the loop to start a new block of moved lines. moved_symbol will be updated at the end of the loop. >> >> if (o->color_moved == COLOR_MOVED_PLAIN) { >> - last_symbol = l->s; >> l->flags |= DIFF_SYMBOL_MOVED_LINE; >> continue; >> } > > I want to make sure that I understand why the `last_symbol` assignment > could be removed without any `moved_symbol` assignment in its place. But I > don't, I still do not see why we do not need a `moved_symbol = l->s;` > assignment here. I had to think about it but I think the answer is that COLOR_MOVED_PLAIN does not care about moved_symbol - it is only used by the zebra coloring modes. > Unless, that is, we extended the `!match` condition above to also cover > the case where `l->s != moved_symbol`. > >> @@ -1251,11 +1252,16 @@ static void mark_color_as_moved(struct diff_options *o, >> &pmb, &pmb_alloc, >> &pmb_nr); >> >> - if (contiguous && pmb_nr && last_symbol == l->s) >> + if (contiguous && pmb_nr && moved_symbol == l->s) >> flipped_block = (flipped_block + 1) % 2; > > This is totally not your fault, but I really wish we could have the much > simpler and much easier to understand `flipped_block = !flipped_block` > here. It's partially my fault - I should have simplified it when I moved that line in b0a2ba4776 ("diff --color-moved=zebra: be stricter with color alternation", 2018-11-23) >> else >> flipped_block = 0; >> >> + if (pmb_nr) >> + moved_symbol = l->s; >> + else >> + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; >> + This is where we update moved_symbol when it did not match l->s above. block_length = 0; >> } >> >> @@ -1265,7 +1271,6 @@ static void mark_color_as_moved(struct diff_options *o, >> if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS) >> l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT; >> } >> - last_symbol = l->s; > > That makes sense: we only set `moved_symbol` when `pmb_nr` had been 0 now, > and don't want it to be overridden. > > As I said, I do not quite understand this patch yet, and am looking for > your guidance to wrap my head around it. > > Thank you for working on this! Thanks for looking at it, I hope these comments help, let me know if I've failed to explain well enough. Best Wishes Phillip > Dscho > >> } >> adjust_last_block(o, n, block_length); >> >> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh >> index 4e0fd76c6c5..15782c879d2 100755 >> --- a/t/t4015-diff-whitespace.sh >> +++ b/t/t4015-diff-whitespace.sh >> @@ -1514,6 +1514,71 @@ test_expect_success 'zebra alternate color is only used when necessary' ' >> test_cmp expected actual >> ' >> >> +test_expect_success 'short lines of opposite sign do not get marked as moved' ' >> + cat >old.txt <<-\EOF && >> + this line should be marked as moved >> + unchanged >> + unchanged >> + unchanged >> + unchanged >> + too short >> + this line should be marked as oldMoved newMoved >> + this line should be marked as oldMovedAlternate newMoved >> + unchanged 1 >> + unchanged 2 >> + unchanged 3 >> + unchanged 4 >> + this line should be marked as oldMoved newMoved/newMovedAlternate >> + EOF >> + cat >new.txt <<-\EOF && >> + too short >> + unchanged >> + unchanged >> + this line should be marked as moved >> + too short >> + unchanged >> + unchanged >> + this line should be marked as oldMoved newMoved/newMovedAlternate >> + unchanged 1 >> + unchanged 2 >> + this line should be marked as oldMovedAlternate newMoved >> + this line should be marked as oldMoved newMoved/newMovedAlternate >> + unchanged 3 >> + this line should be marked as oldMoved newMoved >> + unchanged 4 >> + EOF >> + test_expect_code 1 git diff --no-index --color --color-moved=zebra \ >> + old.txt new.txt >output && cat output && >> + grep -v index output | test_decode_color >actual && >> + cat >expect <<-\EOF && >> + <BOLD>diff --git a/old.txt b/new.txt<RESET> >> + <BOLD>--- a/old.txt<RESET> >> + <BOLD>+++ b/new.txt<RESET> >> + <CYAN>@@ -1,13 +1,15 @@<RESET> >> + <BOLD;MAGENTA>-this line should be marked as moved<RESET> >> + <GREEN>+<RESET><GREEN>too short<RESET> >> + unchanged<RESET> >> + unchanged<RESET> >> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as moved<RESET> >> + <GREEN>+<RESET><GREEN>too short<RESET> >> + unchanged<RESET> >> + unchanged<RESET> >> + <RED>-too short<RESET> >> + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved<RESET> >> + <BOLD;BLUE>-this line should be marked as oldMovedAlternate newMoved<RESET> >> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> >> + unchanged 1<RESET> >> + unchanged 2<RESET> >> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMovedAlternate newMoved<RESET> >> + <BOLD;YELLOW>+<RESET><BOLD;YELLOW>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> >> + unchanged 3<RESET> >> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved<RESET> >> + unchanged 4<RESET> >> + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> >> + EOF >> + test_cmp expect actual >> +' >> + >> test_expect_success 'cmd option assumes configured colored-moved' ' >> test_config color.diff.oldMoved "magenta" && >> test_config color.diff.newMoved "cyan" && >> -- >> gitgitgadget >> >>
Hi Phillip, On Mon, 22 Nov 2021, Phillip Wood wrote: [... a good explanation...] > On 22/11/2021 14:18, Johannes Schindelin wrote: > > > As I said, I do not quite understand this patch yet, and am looking > > for your guidance to wrap my head around it. > > Thanks for looking at it, I hope these comments help, let me know if I've > failed to explain well enough. Yes, thank you, I think I understand enough now to say that the patch looks good to me. Thanks, Dscho
diff --git a/diff.c b/diff.c index 53f0df75329..efba2789354 100644 --- a/diff.c +++ b/diff.c @@ -1176,7 +1176,7 @@ static void mark_color_as_moved(struct diff_options *o, struct moved_block *pmb = NULL; /* potentially moved blocks */ int pmb_nr = 0, pmb_alloc = 0; int n, flipped_block = 0, block_length = 0; - enum diff_symbol last_symbol = 0; + enum diff_symbol moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; for (n = 0; n < o->emitted_symbols->nr; n++) { @@ -1202,7 +1202,7 @@ static void mark_color_as_moved(struct diff_options *o, flipped_block = 0; } - if (!match) { + if (pmb_nr && (!match || l->s != moved_symbol)) { int i; if (!adjust_last_block(o, n, block_length) && @@ -1219,12 +1219,13 @@ static void mark_color_as_moved(struct diff_options *o, pmb_nr = 0; block_length = 0; flipped_block = 0; - last_symbol = l->s; + } + if (!match) { + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; continue; } if (o->color_moved == COLOR_MOVED_PLAIN) { - last_symbol = l->s; l->flags |= DIFF_SYMBOL_MOVED_LINE; continue; } @@ -1251,11 +1252,16 @@ static void mark_color_as_moved(struct diff_options *o, &pmb, &pmb_alloc, &pmb_nr); - if (contiguous && pmb_nr && last_symbol == l->s) + if (contiguous && pmb_nr && moved_symbol == l->s) flipped_block = (flipped_block + 1) % 2; else flipped_block = 0; + if (pmb_nr) + moved_symbol = l->s; + else + moved_symbol = DIFF_SYMBOL_BINARY_DIFF_HEADER; + block_length = 0; } @@ -1265,7 +1271,6 @@ static void mark_color_as_moved(struct diff_options *o, if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS) l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT; } - last_symbol = l->s; } adjust_last_block(o, n, block_length); diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 4e0fd76c6c5..15782c879d2 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -1514,6 +1514,71 @@ test_expect_success 'zebra alternate color is only used when necessary' ' test_cmp expected actual ' +test_expect_success 'short lines of opposite sign do not get marked as moved' ' + cat >old.txt <<-\EOF && + this line should be marked as moved + unchanged + unchanged + unchanged + unchanged + too short + this line should be marked as oldMoved newMoved + this line should be marked as oldMovedAlternate newMoved + unchanged 1 + unchanged 2 + unchanged 3 + unchanged 4 + this line should be marked as oldMoved newMoved/newMovedAlternate + EOF + cat >new.txt <<-\EOF && + too short + unchanged + unchanged + this line should be marked as moved + too short + unchanged + unchanged + this line should be marked as oldMoved newMoved/newMovedAlternate + unchanged 1 + unchanged 2 + this line should be marked as oldMovedAlternate newMoved + this line should be marked as oldMoved newMoved/newMovedAlternate + unchanged 3 + this line should be marked as oldMoved newMoved + unchanged 4 + EOF + test_expect_code 1 git diff --no-index --color --color-moved=zebra \ + old.txt new.txt >output && cat output && + grep -v index output | test_decode_color >actual && + cat >expect <<-\EOF && + <BOLD>diff --git a/old.txt b/new.txt<RESET> + <BOLD>--- a/old.txt<RESET> + <BOLD>+++ b/new.txt<RESET> + <CYAN>@@ -1,13 +1,15 @@<RESET> + <BOLD;MAGENTA>-this line should be marked as moved<RESET> + <GREEN>+<RESET><GREEN>too short<RESET> + unchanged<RESET> + unchanged<RESET> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as moved<RESET> + <GREEN>+<RESET><GREEN>too short<RESET> + unchanged<RESET> + unchanged<RESET> + <RED>-too short<RESET> + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved<RESET> + <BOLD;BLUE>-this line should be marked as oldMovedAlternate newMoved<RESET> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> + unchanged 1<RESET> + unchanged 2<RESET> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMovedAlternate newMoved<RESET> + <BOLD;YELLOW>+<RESET><BOLD;YELLOW>this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> + unchanged 3<RESET> + <BOLD;CYAN>+<RESET><BOLD;CYAN>this line should be marked as oldMoved newMoved<RESET> + unchanged 4<RESET> + <BOLD;MAGENTA>-this line should be marked as oldMoved newMoved/newMovedAlternate<RESET> + EOF + test_cmp expect actual +' + test_expect_success 'cmd option assumes configured colored-moved' ' test_config color.diff.oldMoved "magenta" && test_config color.diff.newMoved "cyan" &&