Skip to content

Commit a7c207b

Browse files
Copilotjongalloway
andauthored
Address review: cache WireframePalette per theme, fix namespace ref, fix inline badge whitespace edge case
Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com> Agent-Logs-Url: https://github.com/jongalloway/DiagramForge/sessions/04fdb593-36fa-43ae-a853-641cef1743a0
1 parent 93e08df commit a7c207b

4 files changed

Lines changed: 35 additions & 4 deletions

File tree

src/DiagramForge/Layout/DefaultLayoutEngine.Wireframe.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using DiagramForge.Models;
2+
using DiagramForge.Parsers.Wireframe;
23
using DiagramForge.Rendering;
34

45
namespace DiagramForge.Layout;
@@ -50,7 +51,7 @@ private static void LayoutWireframeDiagram(Diagram diagram, Theme theme, double
5051
if (diagram.Nodes.Count == 0)
5152
return;
5253

53-
if (!diagram.Nodes.TryGetValue(Parsers.Wireframe.WireframeDslParser.RootNodeId, out var root))
54+
if (!diagram.Nodes.TryGetValue(WireframeDslParser.RootNodeId, out var root))
5455
return;
5556

5657
// Build parent→children map from wireframe:containment edges.

src/DiagramForge/Parsers/Wireframe/WireframeDslParser.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,9 @@ private static bool TrySplitInlineBadge(string line, out string badgeLabel, out
511511

512512
badgeLabel = line[2..closeIdx].Trim();
513513
trailingText = line[(closeIdx + 2)..].Trim();
514-
return badgeLabel.Length > 0;
514+
// Require non-empty trailing text so a standalone badge with only trailing
515+
// whitespace (e.g. "(( Info )) ") is not treated as an inline badge row.
516+
return badgeLabel.Length > 0 && trailingText.Length > 0;
515517
}
516518

517519
private static Node? ParseTabBar(string line, string nodeId)

src/DiagramForge/Rendering/SvgNodeWriter.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Runtime.CompilerServices;
12
using System.Text;
23
using DiagramForge.Models;
34

@@ -747,7 +748,7 @@ private static void AppendClassNode(
747748
/// <see cref="Theme.BackgroundColor"/>, <see cref="Theme.TextColor"/>,
748749
/// <see cref="Theme.AccentColor"/>, and <see cref="Theme.SubtleTextColor"/>.
749750
/// </summary>
750-
private readonly record struct WireframePalette
751+
private sealed record WireframePalette
751752
{
752753
public string CardFill { get; init; }
753754
public string CardBorder { get; init; }
@@ -825,9 +826,17 @@ public static WireframePalette FromTheme(Theme theme)
825826
}
826827
}
827828

829+
/// <summary>
830+
/// Cache keyed by <see cref="Theme"/> identity so that <see cref="WireframePalette.FromTheme"/>
831+
/// is computed at most once per distinct theme per process lifetime.
832+
/// <see cref="ConditionalWeakTable{TKey,TValue}"/> is used so cached entries are automatically
833+
/// discarded when the theme object is GC-collected.
834+
/// </summary>
835+
private static readonly ConditionalWeakTable<Theme, WireframePalette> s_paletteCache = new();
836+
828837
private static void AppendWireframeNode(StringBuilder sb, Node node, string kind, Theme theme)
829838
{
830-
var p = WireframePalette.FromTheme(theme);
839+
var p = s_paletteCache.GetValue(theme, static t => WireframePalette.FromTheme(t));
831840

832841
switch (kind)
833842
{

tests/DiagramForge.Tests/Parsers/WireframeDslParserTests.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,4 +577,23 @@ public void Parse_StandaloneBadge_StillWorks()
577577
Assert.NotNull(badge);
578578
Assert.Equal("Info", badge!.Label.Text);
579579
}
580+
581+
[Fact]
582+
public void Parse_StandaloneBadge_WithTrailingWhitespace_IsNotTreatedAsInline()
583+
{
584+
// A badge followed only by whitespace should NOT create an implicit row.
585+
var diagram = _parser.Parse("wireframe\n(( Info )) ");
586+
587+
// No implicit row node should be created.
588+
var row = diagram.Nodes.Values.FirstOrDefault(n =>
589+
n.Metadata.TryGetValue("wireframe:kind", out var k) && k is "row"
590+
&& !n.Metadata.ContainsKey("wireframe:isRoot"));
591+
Assert.Null(row);
592+
593+
// The badge should still be parsed as a standalone badge.
594+
var badge = diagram.Nodes.Values.FirstOrDefault(n =>
595+
n.Metadata.TryGetValue("wireframe:kind", out var k) && k is "badge");
596+
Assert.NotNull(badge);
597+
Assert.Equal("Info", badge!.Label.Text);
598+
}
580599
}

0 commit comments

Comments
 (0)