Skip to content

Commit a317a45

Browse files
committed
fix(diagrams): #104 review — an emoji is measured as wide as it was meant to be; a dead store goes
Two findings from the code-quality review. Both are right. theme.js, charEm() — "the condition c > 0xffff is always false". The width estimate used where there is no canvas asked "CJK and other full-width scripts" (U+2E80 and up, 1.0 em) before "emoji / astral" (above U+FFFF, 1.1 em). Every code point above U+FFFF is above U+2E80 too, so the second question was never reached and an emoji was measured at 1.0 em. The estimate is meant to err wide — the failure that matters is text overflowing its box — so the questions are now asked the other way round. Nothing on screen changes. The chat measures with the real font, and no fixture contains a character beyond the basic plane, so no snapshot moves. What changes is the fallback when the canvas refuses the font, and the estimates made in Node. layout.js — "the value assigned to span here is unused". When several connectors share a side too short for them, the node grows to hold exactly what they need; span was recomputed afterwards and never read. The recompute is gone and span is a const. it.cs, set on the same line, IS read later, and stays. 6,000 random layouts and the nine gallery diagrams are byte-identical to the code before. diagramLayout gains a test for the estimate's classes: narrow, ordinary and wide letters, digits and capitals, a CJK character at a full em, an emoji at 1.1 em and counted as one character. It fails on the old order, on exactly the condition the review named. 62 suite files pass on macOS and in a Linux container.
1 parent db68f68 commit a317a45

4 files changed

Lines changed: 17 additions & 4 deletions

File tree

‎docs/RICH-DIAGRAMS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ on the default models, then use it for a week and read the counters.
391391

392392
Verified:
393393

394-
- 12 suites, 251 tests (`test/diagram*.test.js`), including a 1,500-spec layout fuzz, the corpus,
394+
- 12 suites, 252 tests (`test/diagram*.test.js`), including a 1,500-spec layout fuzz, the corpus,
395395
SVG snapshots in both palettes, the gallery at chat-panel widths, the real `runAgent` loop with a
396396
scripted provider, and the host functions sliced out of `extension.js` against a `vscode`
397397
stand-in.

‎extensions/levelcode-ai/diagram/layout.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,15 +381,15 @@
381381
const lane = S.labelLane != null ? S.labelLane : T.edge.line + S.labelGap + 2;
382382
const floor = labelled ? lane : (S.portMin != null ? S.portMin : 6);
383383
let gap = Math.max(S.portGap, labelled ? floor : 0);
384-
let span = portSpan(it.node, it.cs, S);
384+
const span = portSpan(it.node, it.cs, S);
385385
if (n > 1 && (n - 1) * gap > span) {
386386
gap = Math.max(floor, span / (n - 1));
387387
if ((n - 1) * gap > span + 1e-6) {
388388
// Still does not fit: the node grows along its side. ("Renderer only: widen, wrap.")
389389
const need = (n - 1) * gap;
390390
const grow = it.node.shape === 'decision' ? need / 0.5 : it.node.shape === 'actor' ? need / 0.55 : need + 2 * S.portInset;
391391
if (right) { it.node.h = Math.ceil(grow); } else { it.node.w = Math.ceil(grow); }
392-
it.cs = cs(it.node); span = portSpan(it.node, it.cs, S);
392+
it.cs = cs(it.node); // (the node now holds exactly `need`: there is nothing left to re-measure)
393393
}
394394
}
395395
ends.forEach((en, i) => {

‎extensions/levelcode-ai/diagram/theme.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@
178178
const c = ch.codePointAt(0) || 0;
179179
if (c >= 0x30 && c <= 0x39) { return 0.60; } // digits
180180
if (c >= 0x41 && c <= 0x5a) { return 0.69; } // A–Z
181+
if (c > 0xffff) { return 1.1; } // emoji and everything else beyond the basic plane — asked first: all of it is above U+2E80 too
181182
if (c >= 0x2e80) { return 1.0; } // CJK and other full-width scripts
182-
if (c > 0xffff) { return 1.1; } // emoji / astral
183183
return 0.57;
184184
}
185185
/**

‎extensions/levelcode-ai/test/diagramLayout.test.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,19 @@ test('PORTS: connectors that share a side each get their own point on it — and
305305
assert.ok(byId(big, 'h').h > byId(big, 't0').h, 'the hub grew along its side');
306306
});
307307

308+
test('MEASURE: the estimate used where there is no canvas errs WIDE, class by class — an emoji is wider than a CJK character', () => {
309+
// Regular weight at 11.5px, so the numbers are plain multiples of the em.
310+
const px = (t) => theme.approxMeasure(t, 'sub');
311+
assert.ok(px('i') < px('a') && px('a') < px('m'), 'narrow, ordinary and wide letters');
312+
assert.ok(px('7') > px('a') && px('A') > px('7'), 'digits and capitals are wider than lower case');
313+
assert.strictEqual(px('\u65e5'), 11.5, 'a CJK character is a full em');
314+
assert.strictEqual(px('\u{1f680}'), 12.7, 'an emoji is 1.1 em — the widest class, not lumped in with CJK');
315+
assert.ok(px('\u{1f680}') > px('\u65e5'));
316+
assert.strictEqual(px('\u{1f680}\u{1f680}'), 25.3, 'and it is ONE character, though it is two code units');
317+
assert.strictEqual(px('\u{20000}'), 12.7, 'anything else beyond the basic plane is measured the same generous way');
318+
assert.strictEqual(px(''), 0); assert.strictEqual(px(null), 0);
319+
});
320+
308321
test('SHAPES: size comes from the measured text — and a connector stops ON a diamond\'s outline', () => {
309322
const double = (t, role) => theme.approxMeasure(t, role) * 2;
310323
const s = R.prepare({ title: 'T', nodes: [{ id: 'a', label: 'A fairly long label' }], edges: [] }).spec;

0 commit comments

Comments
 (0)