Skip to content

Commit 6a19dec

Browse files
clinteckerclaude
andcommitted
Layered edge labels: grow canvas + fit self-loop captions; harden linter + tests
Address PR #6 review feedback (re-verified against d0bac93): - DiagramLayout.swift: the state/class/ER (layered) router finalized its canvas size before placeRunLabels, so a caption on the rightmost/bottom-most run — or one nudged perpendicular off it — could extend past the scene and trip the off-canvas rule. Grow the canvas around the placed label anchors, the same post-placement expansion layoutFlat already does. (chatgpt-codex P2 / coderabbit Major) - DiagramLayout.swift: a labeled self-loop routed a fixed 24pt loop regardless of caption length, cramming the word onto too short a run (label-crowds-edge, e.g. `A --> A: retry`). Grow the loop's vertical return run to the label height plus a stub on each side; keep the top/bottom bars short so a wide loop never gores a neighbouring box (the er fixture's NODE/EDGE pair). - DiagramScene.swift: validate anchorEdge against edges.indices, not just `< count`, so a negative anchor can't trap on the subscript. (coderabbit Major, stability) - EdgeLabelLayoutTests.swift: fail (XCTUnwrap) when a labeled edge has no anchor instead of silently skipping, assert the full flowchartLabelStub (14) rather than a hard-coded 10, and add a layered self-loop lint-clean regression. (coderabbit Minor) Declined the flowchart placeRunLabels "hard-reject runs / drop perpendicular nudges / reserve geometry" rewrite: median-dummy widening already reserves label space, the label-on-fixture/label-crowds-edge linter rules enforce the contract over 30 lint-clean fixtures, and the one real geometry gap (self-loop captions) is fixed above. A wholesale rewrite risks regressing the just-landed placement work with no demonstrated defect. swift build + full swift test green (198 tests, 2 skipped); 30 fixtures lint-clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JoLDcosyaHg3tAKhU5SQaw
1 parent d0bac93 commit 6a19dec

3 files changed

Lines changed: 45 additions & 8 deletions

File tree

Sources/MermaidLayout/DiagramLayout.swift

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -940,18 +940,24 @@ public enum DiagramLayoutEngine {
940940

941941
// Route each edge through its chain waypoints.
942942
var routes: [[CGPoint]] = []
943-
for chain in chains {
943+
for (ci, chain) in chains.enumerated() {
944944
guard chain.count >= 2, let fromFrame = frames[chain[0]], let toFrame = frames[chain[chain.count - 1]] else {
945945
routes.append([.zero, .zero]); continue
946946
}
947947
// Self-loop (an edge from a box back to itself, e.g. an ER
948948
// "subcategory of" parent_id): route it as a small loop off the
949-
// right side, never a straight line through the box interior.
949+
// right side, never a straight line through the box interior. The
950+
// top/bottom bars stay short (a wider loop would gore a neighbour);
951+
// the caption rides the vertical return run, which we grow to the
952+
// label height plus a stub on each side so a word like "retry" isn't
953+
// crammed onto the default span (`label-crowds-edge`).
950954
if chain[0] == chain[chain.count - 1] {
951955
let f = fromFrame
952956
let ext: CGFloat = 24
953-
let yHi = f.midY - min(f.height * 0.24, 13)
954-
let yLo = f.midY + min(f.height * 0.24, 13)
957+
let labelH = (edgeLabelSizes?[ci] ?? nil)?.height ?? 0
958+
let vHalf = max(min(f.height * 0.24, 13), labelH / 2 + flowchartLabelStub)
959+
let yHi = f.midY - vHalf
960+
let yLo = f.midY + vHalf
955961
routes.append([
956962
CGPoint(x: f.maxX, y: yHi),
957963
CGPoint(x: f.maxX + ext, y: yHi),
@@ -1065,7 +1071,7 @@ public enum DiagramLayoutEngine {
10651071

10661072
var routeMaxX = crossExtent + margin
10671073
for pts in routes { for p in pts { routeMaxX = max(routeMaxX, p.x) } }
1068-
let size = CGSize(width: max(crossExtent, routeMaxX - margin) + margin * 2, height: y - layerGap + margin)
1074+
var size = CGSize(width: max(crossExtent, routeMaxX - margin) + margin * 2, height: y - layerGap + margin)
10691075
// Place every caption on the longest clean straight run of its final
10701076
// route, collision-avoiding node boxes, other captions, bends, and
10711077
// crossings — the same run-based placement the flowchart pipeline uses,
@@ -1077,6 +1083,15 @@ public enum DiagramLayoutEngine {
10771083
let labelSizes: [CGSize?] = routingEdges.indices.map { edgeLabelSizes?[$0] ?? nil }
10781084
let labelAnchors = placeRunLabels(routes: routes, labelSizes: labelSizes,
10791085
nodeFrames: realFrameList)
1086+
// Grow the canvas for any caption nudged past the content box — the same
1087+
// post-placement expansion layoutFlat performs — so a label on the
1088+
// rightmost/bottom-most run (or a perpendicular nudge off it) is never
1089+
// clipped or flagged off-canvas.
1090+
for (i, anchor) in labelAnchors.enumerated() {
1091+
guard let lp = anchor, let sz = labelSizes[i] else { continue }
1092+
size.width = max(size.width, lp.x + sz.width / 2 + margin)
1093+
size.height = max(size.height, lp.y + sz.height / 2 + margin)
1094+
}
10801095
// Dummy frames are internal scaffolding — don't leak them.
10811096
for dummy in dummies { frames[dummy] = nil }
10821097
return (frames, size, routes, labelAnchors)

Sources/MermaidLayout/DiagramScene.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ public enum DiagramLayoutLinter {
407407
// Crowding: measure the stub left on each side of the caption on the
408408
// straight run it sits on (the nearest segment of its own route).
409409
for (_, label) in edgeLabels {
410-
guard let ae = label.anchorEdge, ae < scene.edges.count else { continue }
410+
guard let ae = label.anchorEdge, scene.edges.indices.contains(ae) else { continue }
411411
let poly = scene.edges[ae].polyline
412412
guard poly.count >= 2 else { continue }
413413
let c = CGPoint(x: label.frame.midX, y: label.frame.midY)

Tests/MermaidLayoutTests/EdgeLabelLayoutTests.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ final class EdgeLabelLayoutTests: XCTestCase {
5959
guard case .flowchart(let chart) = MermaidParser.parse(src) else { return XCTFail("parse") }
6060
let layout = DiagramLayoutEngine.layout(chart, measure: measure)
6161
for edge in layout.edges {
62-
guard let label = edge.label, !label.isEmpty, let lp = edge.labelPoint else { continue }
62+
guard let label = edge.label, !label.isEmpty else { continue }
63+
let lp = try XCTUnwrap(edge.labelPoint, "\(label): missing label anchor")
6364
let sz = measure(label, DiagramLayoutEngine.labelFontSize)
6465
// Nearest segment (the run the caption sits on).
6566
var run: (a: CGPoint, b: CGPoint, d: CGFloat)?
@@ -79,12 +80,33 @@ final class EdgeLabelLayoutTests: XCTestCase {
7980
let along = horiz ? sz.width + 6 : sz.height + 2
8081
let c = horiz ? lp.x : lp.y
8182
let stub = min((c - along / 2) - lo, hi - (c + along / 2))
82-
XCTAssertGreaterThanOrEqual(stub, 10,
83+
XCTAssertGreaterThanOrEqual(stub, DiagramLayoutEngine.flowchartLabelStub,
8384
"label \"\(label)\" leaves only \(Int(stub))pt of connector")
8485
}
8586
}
8687
}
8788

89+
/// A labeled self-loop in the layered (state/class/ER) router must host its
90+
/// caption without clipping or crowding: the loop is widened to the label
91+
/// plus a stub on each side, and the canvas grows for the label frame. Before
92+
/// the fix, `A --> A: retry` placed the word beside a fixed 24pt loop — off
93+
/// the canvas (no size growth) and then crammed onto too short a run.
94+
func testLayeredSelfLoopLabelsAreClean() {
95+
let sources = [
96+
"stateDiagram-v2\n A --> A: retry",
97+
"classDiagram\n A --> A : self",
98+
"stateDiagram-v2\n [*] --> A\n A --> A: retry\n A --> B: go",
99+
"erDiagram\n CUSTOMER ||--o{ CUSTOMER : refers",
100+
]
101+
for src in sources {
102+
guard let diagram = MermaidParser.parse(src) else { return XCTFail("parse: \(src)") }
103+
let scene = DiagramScene.lower(diagram, measure: measure)
104+
let errors = DiagramLayoutLinter.lint(scene).filter { $0.severity == .error }
105+
XCTAssertTrue(errors.isEmpty,
106+
"self-loop \"\(src)\" not clean:\n" + errors.map { " \($0.kind): \($0.detail)" }.joined(separator: "\n"))
107+
}
108+
}
109+
88110
// MARK: - Linter rejects the OLD (unfixed) geometry
89111

90112
/// `label-crowds-edge`: the OLD short-edge placement centered "records" on a

0 commit comments

Comments
 (0)