Use project .fwlayout files for Avalonia persistence - #1111
Open
johnml1135 wants to merge 15 commits into
Open
Conversation
johnml1135
changed the title
NUnit Tests 1 files ±0 1 suites ±0 8m 1s ⏱️ - 4m 32s For more details on these failures, see this check. Results for commit 7e3eab1. ± Comparison against base commit 2518431. This pull request removes 110 and adds 118 tests. Note that renamed tests count towards both.This pull request removes 2 skipped tests and adds 2 skipped tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
johnml1135
changed the title
johnml1135
marked this pull request as ready for review
August 27, 2026 11:01
Contributor
|
What is the reason PR's 1097 and 1108 could not have gone in before this PR? Would it have significantly changed this PR if those were in first? Please explain why the expectation is that those PR's rebase on this PR. |
mark-sil
reviewed
Aug 27, 2026
mark-sil
left a comment
mark-sil
left a comment
Contributor
There was a problem hiding this comment.
What is the reason PR's 1097 and 1108 could not have gone in before this PR? Would it have significantly changed this PR if those were in first? Please explain why the expectation is that those PR's rebase on this PR.
@mark-sil made 1 comment.
Reviewable status: 0 of 36 files reviewed, all discussions resolved.
This was referenced Aug 27, 2026
johnml1135
force-pushed
the
avalonia-uses-fwlayout
branch
from
August 31, 2026 17:45
eefbcb3 to
8a3637a
Compare
6 tasks
The Avalonia detail view built its own JSON override stack for layout customisation, so changes made there never reached the .fwlayout files the legacy Lexicon Edit view reads. The two views drifted apart, and a project moved between machines lost its Avalonia customisations. Source detail composition from the legacy Inventory instead. A new InventoryViewDefinitionSource turns Inventory layout nodes into view definitions, DetailComposer consumes them, and the Avalonia host reads and writes the project's shared .fwlayout files. Layout commands from the legacy menus now go through the same writers, so a change made in either view shows up in the other and survives a reload. Mirror WinForms layout resolution exactly: the four-field layout identity (class, type, name, choiceGuid) rides every compiled model, composed field, and command target; the named-then-default fallback walks the class chain the way GetTemplateForObjLayout does, including its skip of the concrete class's default; a new Notebook record type clones and persists its choice layout; and Show all right now is a transient reveal that ends when another slice becomes current. Load the same shipped inventories the legacy Inventory loads, DistFiles/Parts and then Language Explorer/Configuration/Parts. The composer had read only the second, so CmObject-Detail-HeavySummary, the generated default layouts, and the autoCustom part did not exist for it and every sense subtree vanished once unresolved parts stopped being recovered. Omit what DataTree omits (unresolved part refs, unrecognised part content) instead of rendering placeholder rows, and shape autoCustom rows from the field's WsSelector like MakeAutoCustomSlice. Retire the now-dead override stack: the applier, differ, editor, JSON serialiser, store, both migrators, and their tests. Record the plan, the WinForms behaviors being mirrored, and the empty divergence register in Docs/architecture/avalonia-fwlayout-parity.md. Cover the new path with layout persistence parity tests, project layout composition tests, identity stability tests, and detail object command execution tests.
Slice.cs's LazySequenceFlid summary described both the flid and the index property that follows it; split so each member states only its own contract, matching LazySequenceIndex's own summary. InventoryViewDefinitionSource.GetSnapshot(string,...) had a caller-framed summary; restate it as the method's own contract (class name resolution, no live object, no CmCustomItem writing-system mapping). Move the "resolve the layout set" comment in DetailComposer.cs from above IsPluralMagicWritingSystem to above ResolveTextRowWritingSystems, which it actually describes. Remove the unused Avalonia.Input import from FwAvalonia/Detail/ DataTree.cs; every reference there is already fully qualified. Fix an over-indented diagnostics.Add call in XmlLayoutImporter.cs's choice-clause branch. Switch the useName != "default" checks in InventoryViewDefinitionSource.GetSnapshot and DetailComposer's CompileForClass from OrdinalIgnoreCase to Ordinal, matching the WinForms reference comparison. Add the sense/record item header row to the fwlayout parity doc's behavior-mapping table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
InventoryViewDefinitionSource.GetSnapshot expanded custom fields on an XElement copy of the resolved layout and, when the placeholder lacked ref="_CustomFieldPlaceholder", persisted that copy. The XmlNode the Inventory had handed out never gained the ref, so any other holder of that node stayed stale and the persisted node was a re-parsed copy rather than the node WinForms EnsureCustomFields would have written. ExpandCustomFields now hands the callback the placeholder that just gained its ref. The source maps it to the live part[@customFields] at the same document-order index, sets the ref on that XmlNode, and persists its nearest layout or part ancestor through Inventory.PersistOverrideElement, which imports the node and replaces the cached entry under the same key. The snapshot copy stays the only thing that grows generated part ref="Custom" siblings, so the project .fwlayout records the placeholder ref alone. Tests cover the live node carrying the ref, the second snapshot leaving the persisted file untouched, and the persisted layout containing no generated Custom parts. Custom-field metadata survives the fixture's per-test undo, so the fixture now clears the static FieldDescription list in teardown and names each test's field uniquely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The parity rule keeps constructs WinForms renders but Avalonia cannot yet
support visible as Unsupported rows, and omits content WinForms itself
omits, reporting it only through import diagnostics. The branch already
applied that to unresolved part refs and to unrecognized part content;
this finishes the remaining XmlLayoutImporter sites, each checked against
DataTree:
- <generate> and unknown container elements: ProcessPartRefNode expands
only sublayout/indent/part (PartGenerator serves browse columns), so
WinForms renders nothing for them.
- <part> without ref, at layout level or injected under obj/seq:
GetMandatoryAttributeValue(partRef, "ref") throws, nothing renders.
- Injected child whose ref cannot be resolved: omitted like any
unresolved part ("Just omit the missing part").
- Non-structural slice content children (deParams, chooserInfo facets,
unknown elements): ProcessSubpartNode ignores them.
- Caller children under a slice part other than <indent>/<part>:
Slice.CreateIndentedNodes consults only the caller's <indent>.
- Non-<part> caller children under an obj/seq part: CreateSlicesFor
unifies them into each item's layout, which the composer reproduces
from SourceCallerXml, so a row here would duplicate that content.
Every diagnostic stays. <if>/<ifnot>/<choice> with an unparseable
condition keep their Unsupported row because WinForms renders content
there. Tests assert the omission per site; the two that named the old
behavior are renamed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review follow-up to 198029c. A bare <part> directly under a slice part's caller was still imported as a child node. Slice.GenerateChildren and CreateIndentedNodes read only caller.SelectSingleNode("indent"), so WinForms never renders such a part; it now falls into the caller-children-dropped branch (reported, omitted). <indent> handling is unchanged. Omitted elements no longer append to the output list, so siblings that shared $"{parentPath}/#{output.Count}" collided on the diagnostic NodePath (a <generate> next to an unknown element, a ref-less injected part next to an unresolvable one). Emitted nodes keep that numbering; every omitted-element diagnostic now goes through OmittedPath, which uses the layout-relative LegacyLayoutCallerPath and falls back to name[ordinal] for part-inventory slice content that lives outside a layout. Tests assert the two paths differ. The two comments the hygiene pass split mid-sentence are single lines again, and the SliceWithUnknownChild fixture drops its editor so the Field-not-Group assertion exercises the promotion boundary it names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EnsureMenuCommandAdapter had no production callers: OnDetailMenuRequested materializes items through CreateNativeDetailMenuItems, and each item's Execute re-targets the hidden adapter tree at click time through EnsureMenuCommandTarget or EnsurePersistentMenuCommandTarget. Delete the wrapper and point the three test fixtures that reached it by reflection at EnsureMenuCommandTarget, which has the same signature. The WinForms adapter-menu fallback origin/main used when native materialization failed is superseded, not lost. It rendered the same xCore ChoiceGroup, so an empty native menu meant an empty WinForms menu, and a WinForms menu would bypass the interceptor and the post-menu refresh that keeps the Avalonia view current. The parity doc now says the native menu is the sole rendering, the stale XCoreMenuBridge note about callers falling back is corrected, and a new test drives OnDetailMenuRequested with a request that resolves to no items to prove the pending Show-all reveal still ends and the detail view refreshes. Fix the DetailObjectCommandExecutionTests class comment so it describes the click-time targeting route instead of a step that no longer exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The parity doc's Divergences register was empty even though the menu paths differ once native construction throws. On origin/main an exception in XCoreMenuBridge conversion fell through to the WinForms adapter ContextMenuStrip (XWindow.ShowContextMenu -> MenuAdapter), so a usable menu still appeared. The Avalonia host now logs the error and shows no menu, because that adapter menu bypasses the bridge interceptor: its commands skip the exact-slice re-targeting and the post-command recompose and leave the view stale. Record that entry, and sharpen plan item 1 so it says logging happens only on the exception path while the zero-items case stays silent. Add a test that drives OnDetailMenuRequested with an in-string request whose context menu id XWindow cannot resolve, so the bridge throws. It asserts the failure reaches the log, the pending Show-all reveal ends, and the hosted detail model recomposes, matching the zero-items test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
InventoryViewDefinitionSource.GetSnapshot and DetailComposer.CompileForClass each carried a copy of the WinForms GetTemplateForObjLayout fallback walk: try the requested name up the class chain, restart at the concrete class's base with "default" once CmObject is reached, throw when that search also reaches CmObject, guard against a metadata cycle, and record the base-class map for part resolution. Two copies could drift, and the parity plan (item 4) wants one algorithm. LayoutResolutionWalk.Resolve now owns the walk. Each caller supplies only its lookup: the inventory source looks up the live Inventory node and folds the RnGenericRec clone-and-persist step into the delegate so it still runs per class visited; the shipped-file compiler looks up its layout index. Both maps are now case-insensitive, which is safe because ViewDefinitionSourceSnapshot already copies them into a case-insensitive dictionary. CompileForClass's "No exact layout found" wording becomes the shared "No matching layout found"; nothing asserted on the old text. Review follow-ups from the placeholder-persistence work: the snapshot copy and the live inventory node now select placeholders through one shared DetailComposer.IsCustomFieldPlaceholder predicate instead of two filters that could drift, and the persisted-file test asserts on parsed part attributes rather than raw substrings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dropping the process-wide (class, layout) memo made every CompileForObject call during ONE compose build a fresh ViewDefinitionSourceSnapshot and SHA-256 the ~300 KB parts XML before the compiler cache could hit, once per sense, example, allomorph, menu-binding peek, sublayout and embedded view. Composing the test entry fingerprinted 9 snapshots with two senses and 16 after adding three more senses with examples and an allomorph. ComposeState now memoizes compiled models per compose, keyed like the existing item-menu-binding memo on (ClassID, layout, choiceGuid, callerXml), so later items of an already-compiled class reuse the model. The memo dies with the compose, so an edited .fwlayout still recompiles on the next one. The snapshot fingerprint also hashes the parts string once per instance via a ConditionalWeakTable and folds that digest into the layout hash, so each remaining fingerprint costs one small hash plus a lookup. Identical content still yields identical keys and any content change still changes the key. Internal compile/fingerprint/parts-hash counters, exposed to xWorksTests and FwAvaloniaTests through a new InternalsVisibleTo, let the tests assert the bound instead of timing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The per-compose memo of compiled models (and the item-menu-binding memo it mirrors) keyed on (ClassID, layout, choiceGuid, callerXml), assuming the source reads nothing from an object beyond its class. It does read one more thing: InventoryViewDefinitionSource.GetSnapshot maps a CmCustomItem's layout name from its OWNING LIST's WsSelector, so two custom items of one class from lists with different selectors composed in one pass (a possibility-reference descent, for example) shared one memo entry and the second item rendered the first item's layout. Both keys now carry a resolver discriminator computed by one helper: the owning list's Hvo for an ICmCustomItem (0 when unowned) and 0 for every other class. A new ProjectLayoutCompositionTests case composes a custom item whose Restrictions reference an item from an analysis-selector list and one from a vernacular-selector list and checks each gets its own CmPossibilityA / CmPossibilityV marker layout; it failed before this change with the second item showing the first item's marker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Divergences entry described how the Avalonia host used to behave before this work, which says nothing about the WinForms contract it is measured against; the entry now states WinForms behavior alone and leaves the rationale to "Why accepted". LayoutResolutionWalk's summary named the two callers instead of stating the walk's own contract, and its exception list omitted the argument checks. Three menu tests each repeated the same eight-line Show-all arrangement and two repeated the OnDetailMenuRequested reflection lookup; both now come from one helper apiece. The log message the failure test asserts on is now a constant shared with the production call site, and the no-menu test asserts the other half of the documented contract: a request that simply resolves to no items logs nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The parity document described its steps as work to do to a pull request, though it outlives that request and describes how the detail view is built. State the design instead. Nothing said why LayoutNotFoundException is rethrown past the handler that falls back to the host view two lines below, so the clause reads as redundant. A layout the class hierarchy cannot satisfy means a corrupt project, which WinForms also reports rather than hides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
johnml1135
force-pushed
the
avalonia-uses-fwlayout
branch
from
September 5, 2026 17:40
8a3637a to
2a419c4
Compare
Both operands of the refreshAfterMenu expression consume pending state, so || would leave the Show-all reveal on the previous slice whenever a focus refresh was already pending. Nothing said so, inviting a future edit to "fix" the operator. The compile counters described themselves by the tests that read them rather than by what they count, and RecordEditView.Avalonia.cs had lost its byte-order mark.
Two changes rode along with fwlayout persistence without being needed by it, so they are moving to branches of their own. The compiler swapped its cache for a capacity-bounded one with Lazy-based deduplication, memoized the parts-source hash, and counted compiles and fingerprints so a test could bound them. Nothing measured said the old cache was a problem. The simple locked dictionary comes back, and the counters stop shipping in production code. The per-compose memo in the composer stays, since it is what stops one compose recompiling a layout per item. Restricting a row to its configured writing systems also kept any unselected alternative that happened to hold data. That is a display rule in its own right, not something layout parity asks for, so the row now shows exactly the configured set. Both live on avalonia-viewdef-compile-cache and avalonia-ws-alternatives-with-data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The design section said both menu outcomes were registered as divergences, but only the throwing one was. WinForms opens its adapter menu even when it holds no items, so an empty popup appears where Avalonia now opens nothing. 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
3 participants
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.
Hide a field in the Avalonia detail view, move one, or configure its writing
systems, and the change now lands in the project's
.fwlayoutfile — the samefile the legacy Lexicon Edit view reads. Do it in either view and the other
one shows it, and it survives moving the project to another machine.
Until now the Avalonia view kept those settings in a
.viewoverride.jsonstoreof its own, so the two views drifted apart and a moved project lost its
Avalonia customizations. That store is deleted here (20 files, 3,313 lines);
nothing in
Srcreferences it any more. Existing.viewoverride.jsonfilesare ignored, not migrated — the format never shipped in a release.
The question worth your time is not "does it write the file" — tests cover
that — but whether a layout that WinForms would render now renders the
same. Composition, fallback, and command targeting were all rebuilt against
the legacy
XCore.Inventory, and the contract they must meet is written downin
Docs/architecture/avalonia-fwlayout-parity.md.Where to look
InventoryViewDefinitionSource.cs— the first Avalonia code that writesinto a live project
.fwlayout. MirrorsDataTree.EnsureCustomFields.LayoutResolutionWalk.cs— WinForms fallback order, including the quirkthat the
defaultsearch starts at the concrete class's base.RecordEditView.Avalonia.cs— largest rewrite; holds the overrideretirement and the fail-closed command targeting.
XmlLayoutImporter.cs— where matching WinForms makes the new viewrender less than the previous Avalonia code did.
DetailControls/{DataTree,Slice}.cs— the only WinForms productionfiles touched; additive virtuals, no existing path changes.
Deliberately not here
avalonia-viewdef-compile-cache(compile-cache rewrite — unmeasured, may be dropped) and
avalonia-ws-alternatives-with-data(fix for LT-22777).avalonia-importer-winforms-strictness,avalonia-layout-choice-fail-closed,avalonia-custom-field-placeholder-persistence.otherwise empty.
Verification
xWorks 1,657/1,659 and FwAvalonia 637/638 pass locally (remainder skipped);
comment hygiene, token hygiene and gitlint clean. No manual FLEx run against a
real project — the parity claims rest on tests and on reading the WinForms
sources.
Reading this a year from now — start here
The design contract for this work is durable and lives in the tree, at
Docs/architecture/avalonia-fwlayout-parity.md. It carries the four-fieldlayout identity, the WinForms behaviors being matched, the acceptance
criteria, and the
Divergencesregister — the list of places Avalonia isallowed to differ. If you are changing detail-view layout behavior, read it
first: anything not in that register that differs from WinForms is a defect.
What is not in the tree, and lives only here, is why the scope is what it
is: what was tried, what was cut, and what was left in on purpose.
The layer cake
Inventorystays the only thing that loads, merges, and persists layout XML.FwAvalonia never depends on XCore and never holds a mutable inventory node;
the snapshot is the seam.
Layout identity is the four attributes
Inventoryitself keys on —class,type,name,choiceGuid— plus a caller path locating the exact composedoccurrence. A command targets an already-resolved layout; it never re-runs
fallback.
Decisions, and why
Both shipped parts directories are loaded, hand-authored first.
InventoryloadsDistFiles/Partsand thenLanguage Explorer/Configuration/Parts, letting the later files replacesame-id entries.
LayoutSourceLoaderis first-wins, so the same precedenceis expressed by listing the hand-authored directory first. This matters more
than it looks:
CmObject-Detail-HeavySummary, theautoCustompart, andevery generated
defaultlayout live only inDistFiles/Parts. Readingone directory silently empties every sense subtree.
Omit what WinForms omits. An unresolved part ref, or part content
DataTree.ProcessSubpartNodedoes not recognize, produces a diagnostic and norow — because that is what the legacy view does. Rendering a visible
"unsupported" placeholder there would itself be a divergence. Constructs
WinForms does render but Avalonia cannot yet support still show as
unsupported rows.
Fail closed on command targets. A persistent command resolves to one
exact hidden WinForms slice by object, field, class, layout, and caller path.
Zero matches or several: the command is disabled and logged. It never falls
back to a nearby occurrence to make itself succeed.
One walk, not two. The fallback algorithm is generic over the live
inventory node and the shipped-file element, so the two resolution paths
cannot drift.
Paths not taken
Migrating
.viewoverride.json. Rejected. The format is pre-alpha, hiddenbehind a flag, and never shipped in a release. Conversion code would have to
be maintained and eventually deleted anyway; the files are simply inert now.
Keeping both stores in sync. Rejected as the thing that caused the bug.
A committed canonical-JSON snapshot as the load path. Removed. A snapshot
baked at build time cannot reflect a project file a user just edited, which is
the entire point of this work.
Bounding the compile cache here. Moved to
avalonia-viewdef-compile-cache.The capacity bound,
Lazy-based deduplication, memoized parts hash, andinstrumentation counters were precautionary: nothing measured said the previous
cache was a problem, and the counters were test-only instrumentation shipping
in production code. The per-compose memo did stay — without it a single
compose rebuilt a snapshot per sense.
Keeping unselected writing systems that hold data. Moved to
avalonia-ws-alternatives-with-data. It is a display rule in its own right,not layout parity, and LT-22777 already reports it as a user-facing bug.
What this does NOT authorize
construct. Gaps stay visible as unsupported rows with named TODOs; they are
defects to fix, not approved divergences.
.fwlayoutthe permanent format. The parity doc'sRetirementsection describes replacing it once no supported WinForms pathdepends on it — that migration is unwritten and this branch is not its
precedent.
(
avalonia-importer-winforms-strictness,avalonia-layout-choice-fail-closed,avalonia-custom-field-placeholder-persistence) describe behavior thatshould stay. They exist so each can be judged alone; all three are WinForms
behavior being matched, and
DataTree.EnsureCustomFieldshas written thecustom-field placeholder ref to project files for years.
Evidence
Fallback order —
LayoutResolutionWalk.Resolvewas compared line by lineagainst
DataTree.GetTemplateForObjLayout: requested name up the class chain;at
CmObjectthe name resets todefaultand the class resets to the concreteclass, but the walk advances to that class's base before the next lookup, so
the concrete class's own default is never checked. Covered by
InventoryViewDefinitionSourceTests.The override store is gone —
git grepforViewOverride,viewoverride, andViewDefinitionOverrideacrossSrcreturns nothing.Custom-field placeholder — the ref is set on the live
XmlNodetheInventoryhanded out, then persisted throughPersistOverrideElement,matching
EnsureCustomFields. Generatedref="Custom"siblings are notpersisted; WinForms regenerates them per load.
Notebook record types — a missing
RnGenericRecchoice layout is clonedfrom the no-choice layout, tagged, added to the inventory, and written to the
project file, asserted against a real
.fwlayoutwrite.Show-all — the transient reveal ends when another slice becomes current
and cannot survive a record or Type change; both paths have tests, including
the case where no menu can be shown at all.
This change is