Skip to content

Commit 84a1704

Browse files
Geeven SinghCopilot
andcommitted
Paint the full line strong when there are no intra-line spans
Soft line tints (RemovedLineBackground / AddedLineBackground) are only the right base layer when the intra-line colorizer is about to overlay strong red/green sub-spans on top -- they narrow the claim from "this whole line changed" to "specifically these characters inside the line". With no spans, the soft tint left the user with a faint "something changed somewhere in this region" signal and no visual emphasis on the changed line at all -- the exact failure mode the previous yellow-with-no-spans fix targeted, just in a different color. The new rule is uniform: any line whose highlight has no intra-line spans gets the strong intra-line brush across the full row; only Modified (which by DiffHighlightMap contract always carries spans) keeps the soft yellow base layer for the colorizer to paint over. This applies to: - Pure inserts and pure deletes (no opposing side to compare against). - Paired delete+insert lines under intra-line off (no pairing happens, both sides spill as Deleted/Inserted). - Paired delete+insert lines under intra-line on that the pairing- viability policy demoted to unpaired (the demote path emits null spans on purpose). Implementation: brush selection extracted to a shared internal LineBackgroundBrushSelector that both DiffBackgroundRenderer (side- by-side) and InlineDiffBackgroundRenderer (inline) call into, with unit tests covering every (DiffSide, DiffLineKind) cell. The previous inline implementation drifted slightly from side-by-side (it didn't need a side discriminator); the shared selector is the only place the rule lives now, so future scheme tweaks stay consistent across both render modes. Side benefit: the side-by-side and inline line tints now match the strong colors the hunk overview bar already uses for its mini-map markers, so the rendering story is consistent vertically too. AI-Local-Session: 4519f6b6-393a-4476-8efa-410e5396c3a9 AI-Cloud-Session: 72f9e474-60ab-42c2-b2a0-28fee827cbbb Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fc16b5e commit 84a1704

5 files changed

Lines changed: 166 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ body. Keep section headings exact and write notes in Markdown.
1212

1313
## [Unreleased]
1414

15+
### Changed
16+
17+
- **Lines with no intra-line spans now paint with the strong
18+
(intra-line) tint across the full row, not the soft line tint.**
19+
Previously, pure inserts/deletes and paired-but-not-shown changes
20+
(intra-line off, or paired lines the similarity policy demoted)
21+
rendered with the same faint background as everything else in the
22+
hunk — leaving the user with a subtle "something changed" signal
23+
but no visual emphasis on the changed line itself. The new rule is
24+
uniform: if there are no inner red/green spans, the whole row gets
25+
the strong tint ("all of this is the change"); if there are spans,
26+
the soft yellow ModifiedLineBackground stays as the base layer and
27+
the colorizer overlays the strong spans on top. This applies in
28+
side-by-side and inline modes, and matches the strong colors the
29+
hunk overview bar already uses for its mini-map markers.
30+
1531
### Fixed
1632

1733
- **Paired delete+insert lines no longer render as yellow "Modified"
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
using System.Windows.Media;
2+
using DiffViewer.Models;
3+
using DiffViewer.Rendering;
4+
using FluentAssertions;
5+
using Xunit;
6+
using DiffSide = DiffViewer.Rendering.DiffSide;
7+
8+
namespace DiffViewer.Tests.Rendering;
9+
10+
public sealed class LineBackgroundBrushSelectorTests
11+
{
12+
private static readonly DiffColorScheme Scheme = DiffColorScheme.Classic;
13+
14+
[Fact]
15+
public void Pick_LeftDeleted_ReturnsStrongRemovedBrush()
16+
{
17+
// The whole point of the strong-when-no-spans rule: a Deleted
18+
// line on the left side has, by DiffHighlightMap contract, no
19+
// intra-line spans -- so the brush must be strong, not soft.
20+
var brush = LineBackgroundBrushSelector.Pick(DiffSide.Left, DiffLineKind.Deleted, Scheme);
21+
22+
brush.Should().BeSameAs(Scheme.RemovedIntraLineBackground);
23+
brush.Should().NotBeSameAs(Scheme.RemovedLineBackground);
24+
}
25+
26+
[Fact]
27+
public void Pick_RightInserted_ReturnsStrongAddedBrush()
28+
{
29+
var brush = LineBackgroundBrushSelector.Pick(DiffSide.Right, DiffLineKind.Inserted, Scheme);
30+
31+
brush.Should().BeSameAs(Scheme.AddedIntraLineBackground);
32+
brush.Should().NotBeSameAs(Scheme.AddedLineBackground);
33+
}
34+
35+
[Fact]
36+
public void Pick_InlineDeleted_ReturnsStrongRemovedBrush()
37+
{
38+
var brush = LineBackgroundBrushSelector.Pick(DiffSide.Inline, DiffLineKind.Deleted, Scheme);
39+
40+
brush.Should().BeSameAs(Scheme.RemovedIntraLineBackground);
41+
}
42+
43+
[Fact]
44+
public void Pick_InlineInserted_ReturnsStrongAddedBrush()
45+
{
46+
var brush = LineBackgroundBrushSelector.Pick(DiffSide.Inline, DiffLineKind.Inserted, Scheme);
47+
48+
brush.Should().BeSameAs(Scheme.AddedIntraLineBackground);
49+
}
50+
51+
[Theory]
52+
[InlineData(DiffSide.Left)]
53+
[InlineData(DiffSide.Right)]
54+
[InlineData(DiffSide.Inline)]
55+
public void Pick_Modified_ReturnsSoftModifiedBrush(DiffSide side)
56+
{
57+
// Modified means DiffHighlightMap also attached intra-line spans;
58+
// the IntraLineColorizer overlays the strong red/green spans on
59+
// top of the soft yellow background, so the soft brush is the
60+
// correct base layer.
61+
var brush = LineBackgroundBrushSelector.Pick(side, DiffLineKind.Modified, Scheme);
62+
63+
brush.Should().BeSameAs(Scheme.ModifiedLineBackground);
64+
}
65+
66+
[Theory]
67+
[InlineData(DiffSide.Left, DiffLineKind.Context)]
68+
[InlineData(DiffSide.Right, DiffLineKind.Context)]
69+
[InlineData(DiffSide.Inline, DiffLineKind.Context)]
70+
[InlineData(DiffSide.Left, DiffLineKind.Inserted)] // wrong side
71+
[InlineData(DiffSide.Right, DiffLineKind.Deleted)] // wrong side
72+
public void Pick_NonTintedCases_ReturnsTransparent(DiffSide side, DiffLineKind kind)
73+
{
74+
var brush = LineBackgroundBrushSelector.Pick(side, kind, Scheme);
75+
76+
brush.Should().BeSameAs(Brushes.Transparent);
77+
}
78+
79+
[Fact]
80+
public void Pick_NullScheme_ThrowsArgumentNullException()
81+
{
82+
var act = () => LineBackgroundBrushSelector.Pick(DiffSide.Left, DiffLineKind.Deleted, null!);
83+
84+
act.Should().Throw<ArgumentNullException>();
85+
}
86+
}

DiffViewer/Rendering/DiffBackgroundRenderer.cs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public enum DiffSide
3333
/// its highlight dictionary from <see cref="LineHighlights"/> on every
3434
/// <see cref="Draw"/>, so updating the map plus calling
3535
/// <c>TextView.Redraw()</c> is enough to repaint.
36+
///
37+
/// <para>Brush selection is delegated to
38+
/// <see cref="LineBackgroundBrushSelector"/>; see its summary for the
39+
/// strong-vs-soft rule.</para>
3640
/// </summary>
3741
public sealed class DiffBackgroundRenderer : IBackgroundRenderer
3842
{
@@ -59,7 +63,7 @@ public void Draw(TextView textView, DrawingContext drawingContext)
5963
int docLine = visualLine.FirstDocumentLine.LineNumber;
6064
if (!LineHighlights.TryGetValue(docLine, out var highlight)) continue;
6165

62-
Brush brush = PickBrush(highlight.Kind);
66+
Brush brush = LineBackgroundBrushSelector.Pick(_side, highlight.Kind, _scheme);
6367

6468
// Paint the full line width so the tint extends to the right edge
6569
// even on short lines; matches GitHub / VS Code line-diff style.
@@ -75,13 +79,4 @@ public void Draw(TextView textView, DrawingContext drawingContext)
7579
}
7680
}
7781
}
78-
79-
private Brush PickBrush(DiffLineKind kind) => (_side, kind) switch
80-
{
81-
(DiffSide.Left, DiffLineKind.Deleted) => _scheme.RemovedLineBackground,
82-
(DiffSide.Left, DiffLineKind.Modified) => _scheme.ModifiedLineBackground,
83-
(DiffSide.Right, DiffLineKind.Inserted) => _scheme.AddedLineBackground,
84-
(DiffSide.Right, DiffLineKind.Modified) => _scheme.ModifiedLineBackground,
85-
_ => Brushes.Transparent,
86-
};
8782
}

DiffViewer/Rendering/InlineDiffBackgroundRenderer.cs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ namespace DiffViewer.Rendering;
1111
/// <see cref="Draw"/> and paints the appropriate brush for that kind.
1212
/// Hunk-header lines and blank separators are absent from the dictionary
1313
/// and rendered without a tint.
14+
///
15+
/// <para>Brush selection is delegated to
16+
/// <see cref="LineBackgroundBrushSelector"/> (with <see cref="DiffSide.Inline"/>);
17+
/// see its summary for the strong-vs-soft rule.</para>
1418
/// </summary>
1519
public sealed class InlineDiffBackgroundRenderer : IBackgroundRenderer
1620
{
@@ -35,13 +39,7 @@ public void Draw(TextView textView, DrawingContext drawingContext)
3539
int docLine = visualLine.FirstDocumentLine.LineNumber;
3640
if (!LineHighlights.TryGetValue(docLine, out var highlight)) continue;
3741

38-
Brush brush = highlight.Kind switch
39-
{
40-
DiffLineKind.Inserted => _scheme.AddedLineBackground,
41-
DiffLineKind.Deleted => _scheme.RemovedLineBackground,
42-
DiffLineKind.Modified => _scheme.ModifiedLineBackground,
43-
_ => Brushes.Transparent,
44-
};
42+
Brush brush = LineBackgroundBrushSelector.Pick(DiffSide.Inline, highlight.Kind, _scheme);
4543

4644
foreach (var rect in BackgroundGeometryBuilder.GetRectsFromVisualSegment(
4745
textView, visualLine, 0, int.MaxValue))
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
using System.Windows.Media;
2+
using DiffViewer.Models;
3+
4+
namespace DiffViewer.Rendering;
5+
6+
/// <summary>
7+
/// Resolves the per-line background brush both
8+
/// <see cref="DiffBackgroundRenderer"/> (side-by-side) and
9+
/// <see cref="InlineDiffBackgroundRenderer"/> (inline) paint.
10+
///
11+
/// <para>The rule is uniform across both modes: <em>any line with no
12+
/// intra-line spans gets the strong intra-line brush; lines with spans
13+
/// get the soft modified tint and the colorizer paints the strong
14+
/// spans on top of it</em>. By contract, <see cref="DiffHighlightMap"/>
15+
/// emits <see cref="DiffLineKind.Modified"/> only when intra-line
16+
/// analysis produced spans, and emits <see cref="DiffLineKind.Deleted"/>
17+
/// / <see cref="DiffLineKind.Inserted"/> with null spans in every other
18+
/// case (pure delete/insert, intra-line off, demote path). The
19+
/// selector keys off <see cref="DiffLineKind"/> alone because that
20+
/// contract makes spans-vs-no-spans equivalent to
21+
/// Modified-vs-Deleted/Inserted.</para>
22+
///
23+
/// <para>Why strong-when-no-spans: the strong brush says "this whole
24+
/// rectangle is what changed"; the soft brush only makes sense when
25+
/// the colorizer is about to overlay strong sub-spans that narrow the
26+
/// claim ("here within the line"). Painting soft without overlay leaves
27+
/// the user with a faint "something changed" signal with no indication
28+
/// of where — which is the bug this rule fixes for the no-spans cases.</para>
29+
/// </summary>
30+
internal static class LineBackgroundBrushSelector
31+
{
32+
public static Brush Pick(DiffSide side, DiffLineKind kind, DiffColorScheme scheme)
33+
{
34+
ArgumentNullException.ThrowIfNull(scheme);
35+
36+
return (side, kind) switch
37+
{
38+
// Strong full-line tints for pure inserts/deletes and for
39+
// paired-but-no-spans (intra-line off or demote path).
40+
(DiffSide.Left, DiffLineKind.Deleted) => scheme.RemovedIntraLineBackground,
41+
(DiffSide.Right, DiffLineKind.Inserted) => scheme.AddedIntraLineBackground,
42+
(DiffSide.Inline, DiffLineKind.Deleted) => scheme.RemovedIntraLineBackground,
43+
(DiffSide.Inline, DiffLineKind.Inserted) => scheme.AddedIntraLineBackground,
44+
45+
// Soft yellow for Modified — the colorizer paints strong
46+
// red/green spans on top so the yellow shows through only in
47+
// the unchanged-within-the-line gaps.
48+
(_, DiffLineKind.Modified) => scheme.ModifiedLineBackground,
49+
50+
// Context lines and anything else: no tint.
51+
_ => Brushes.Transparent,
52+
};
53+
}
54+
}

0 commit comments

Comments
 (0)