Give every fragment's content root its canonical id - #98
Merged
Merged
Conversation
Injecting fragments into the generated classes needs one rule for finding a fragment's content. With canonical ids that rule is "the element with the expected id"; without them it falls back on guessing from position, which fails outright for the fragments whose content was a loose <path> at the root. Every fragment now holds exactly one content element, a <g> carrying the name its family goes by: main, mod1, mod2, frame, frame_overlay, echelon, oca, eng_bar, amplifier, hqtffd. All lowercase - hqtffd loses its capitals with the rest, so there is no exception to remember. Four kinds of change, 649 files: - Renames. Mostly an editor's duplicate suffix - main_1_, mod1_2_, mod2_2_, HQTFFD_1_, main-7 - but also case (Main, Symbol, Mod1, Octagon), two plain typos (mian, ocatgon), and a few that had drifted to another family's name altogether: thirteen mod2 fragments whose group was called mod1, eleven called main. - Wraps. 445 fragments named the content but put the name on a bare <rect>, <path>, <text> or <line>. Each is now inside a group, so one rule covers every family rather than one for OCA and Engagement and another for the rest. - Merges. 70 held content in more than one root element: a duplicate main-7 group, a <path> drawn beside an empty main, or an anonymous group that was plainly the octagon guide and had simply lost its id. - The octagon guide keeps its own name, spelled one way. Nothing is drawn differently, which took three goes to be true. Hoisting a group's children into the canonical group drops the transform or display it was imposing on them, so a group carrying anything but an id now moves intact. Paint order is document order, so what sat before the content goes in at the top of it and what sat after goes in at the bottom - appending everything put a frame that was drawn behind an icon in front of it. And an "empty" group meant an empty <g>, never a leaf element: a <line id="main"> has no children and was read as an empty container and dropped, taking the drawing with it. Verified three independent ways: - Every drawing element compared before and after with its ancestors' attributes folded in and display:none subtrees left out, so a move between groups carrying nothing but an id is invisible to the comparison. 649 of 649 identical here, 717 of 717 in hallux. - FragmentMeasurer re-run over both models: every measured bound identical. - The 2,972 shared fragments still byte-identical across the two trees. Thirteen fragments are left alone and listed on the issue: five that draw nothing at all, and eight where merging would impose a transform, style or display on the content being moved, which is a change to the drawing rather than to the structure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
564 groups across 492 files, and every one of them a <g> with no attributes at all and no siblings: a container around exactly one thing, styling nothing and transforming nothing. Editor leftovers from a copy or an ungroup. Some fragments carried four of them one inside another - Dismounted 27110107 wrapped its one polygon in five groups. Removing them cannot change the drawing by construction. An element's effective attributes are its own plus every ancestor's, and an ancestor with no attributes contributes nothing to that; with no siblings there is no paint order to disturb either. The rendered comparison agrees, 234 of 234 identical, and every measured bound comes back the same. Two things to know if this is ever run again. The bare test has to read the opening tag: a closing tag carries no attributes, so taking it from there calls every element with a separate close tag attribute-free, and the first version of this cheerfully removed content roots. And where these nest, only one can be removed per pass - taking out the inner group moves the outer one's closing tag, so offsets gathered in a single sweep go stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #80.
6,211 fragments across the two trees now hold exactly one content element: a
<g>carrying the name its family goes by —main,mod1,mod2,frame,frame_overlay,echelon,oca,eng_bar,amplifier,hqtffd. All lowercase, so there is no exception to remember. 649 files changed here, 717 in hallux.Injecting fragments into the generated classes needs one rule for finding a fragment's content. With canonical ids that rule is "the element with the expected id"; without them it falls back on guessing from position, which fails outright for the fragments whose content was a loose
<path>at the root.Four kinds of change
main_1_,mod1_2_,HQTFFD_1_,main-7), plus case (Main,Symbol,Mod1,Octagon), two plain typos (mian,ocatgon), and 24 that had drifted to another family's name — mod2 fragments whose group was calledmod1ormain<rect>,<path>,<text>or<line>; each is now inside a group, so one rule covers every family rather than one for OCA and Engagement and another for everything elsemain-7group, a<path>drawn beside an emptymain, or an anonymous group that was plainly the octagon guide and had lost its idOctagon/ocatgonspelled one wayTwo decisions taken along the way, both yours:
amplifierlowercase (52 files went fromAmplifierrather than the 8 going the other way, because every other id is lowercase), andhqtffdlosing its capitals for the same reason.Redundant nested groups
A second commit takes out 564 groups across 492 files — every one a
<g>with no attributes at all and no siblings, so a container around exactly one thing, styling nothing and transforming nothing. Editor leftovers from a copy or an ungroup; Dismounted27110107wrapped its single polygon in five of them.That one cannot change the drawing by construction: an element's effective attributes are its own plus every ancestor's, and an ancestor with no attributes contributes nothing; with no siblings there is no paint order to disturb either. The rendered comparison agrees — 234 of 234 identical — and every measured bound comes back the same.
Nothing is drawn differently — which took three goes
Worth recording, because each was a way to silently lose a drawing:
main-7group withtransform="translate(0,0.4)"lost the shift. A group carrying anything but an id now moves intact.Framethat was drawn behind an icon in front of it. What sat before the content now goes in at the top, what sat after goes in at the bottom.<g>, never a leaf.<line id="main">has no children, was read as an empty container, and was dropped — taking the drawing with it.The first two showed up only in the rendered comparison, not in any count.
Verification
Three independent checks, since
SvgFingerprintcan't guard structural moves:display:nonesubtrees left out, so a move between groups carrying nothing but an id is invisible to the comparison. 649 of 649 identical here, 717 of 717 in hallux (a further 71 hallux files differ from its HEAD because of the uncommitted Echelon amplifiers are half text and half geometry - make them all geometry #68/Control Measure fragments: separate the drawn content from the construction scaffolding #78/Frame fragments: give each a single content root #79 work).FragmentMeasurerre-run over both models: every measured bound identical, no diff at all.Normaliser clean,
mvn verifygreen.Left alone
13 fragments, listed with the reasoning on the issue: five that draw nothing at all, and eight where merging would impose a
transform,styleordisplayon the content being moved — a change to the drawing rather than to the structure, so the pass refused rather than guess.Next
FragmentShapeCheckercurrently covers free canvas fragments only, and its javadoc says this is where it grows. With these names settled it can cover every family.The matching changes to
jmsfx-halluxare in the hallux working tree, uncommitted.🤖 Generated with Claude Code