Regression in highlighting from #1037

[phillipwood]

I noticed that delta has started to sometimes highlight unchanged whitespace before insertions. This appears to be caused by #1037

With delta built from feec45b^ only the insertion is highlighted

With delta built from feec45b the space before the insertion is highlighted

[phillipwood]

Here is the patch for reproducing the issue

diff --git a/file b/file
@@ -1,7  +1,7 @@
    entry = oidmap_get(&state.commit2label, &commit->object.oid);

    if (entry)
-       strbuf_addf(out, "\n%c Branch %s\n", comment_line_char, entry->string);
+       strbuf_addf(out, "\n%s Branch %s\n", comment_line_str, entry->string);
    else
        strbuf_addch(out, '\n');

Unfortunately the markdown parser seems to be trimming the trailing empty line from the diff.

[imbrish]

I managed to narrow it down to a such example:

printf 'diff a a\n@@ -1,1 +1,1 @@\n-x a x b\n+x c x d' | delta

The specific characters do not matter much, they are mostly there to facilitate detection of the changes in line. It does not occur at the first difference but does occur at the second and subsequent ones.

Adding such test to src/edits.rs:

#[test]
fn test_infer_edits_1658() {
    assert_edits(
        vec!["x a x b"],
        vec!["x c x d"],
        (
            vec![vec![
                (MinusNoop, "x "),
                (Deletion, "a"),
                (MinusNoop, " x "),
                (Deletion, "b"),
            ]],
            vec![vec![
                (PlusNoop, "x"),
                (PlusNoop, " "),
                (Insertion, "c"),
                (PlusNoop, " x"),
                (PlusNoop, " "),
                (Insertion, "d"),
            ]],
        ),
        1.0,
    );
}

Results in (emphasis mine):

Not sure why in the plus line the trailing whitespace split into separate tokens, for example x splits into x and . This behavior was introduced in #1037, and it seems the author anticipated that the whitespace before changes would be highlighted, as tests were updated to allow this, like here: https://github.com/dandavison/delta/pull/1037/files#diff-0d3b24c177e6f1af4c40ad3518c8f646cafeb61be14c1c6583093d908a64de5dL786-R835.

I suspect because of that change, there are more tokens in the plus line than in the minus line, and therefore the additional space tokens are highlighted as additions?

[phillipwood]

Not sure why in the plus line the trailing whitespace split into separate tokens, for example x splits into x and . This behavior was introduced in #1037, and it seems the author anticipated that the whitespace before changes would be highlighted, as tests were updated to allow this, like here: https://github.com/dandavison/delta/pull/1037/files#diff-0d3b24c177e6f1af4c40ad3518c8f646cafeb61be14c1c6583093d908a64de5dL786-R835.

The trailing space is split off so that it can be highlighted as a whitespace error if it comes at the end of the line. I think the tests changes in that commit are buggy, I've got a fix that I will post soon.

I suspect because of that change, there are more tokens in the plus line than in the minus line, and therefore the additional space tokens are highlighted as additions?

The total number of tokens shouldn't matter, the problem is the whitespace token gets added with a different operation to the non-whitespace part by a mistake.

[imbrish]

Well spotted! And thanks for the explanations 😊

[dandavison]

Thanks for the investigation @phillipwood and @imbrish -- I've merged @phillipwood's fix.