Skip to content

LT-22691: Answer the Lexeme Form jump, swap and convert leaves natively - #1186

Merged
mark-sil merged 1 commit into
mainfrom
LT-22691o
Oct 15, 2026
Merged

mark-sil merged 1 commit into
mainfrom
LT-22691o

Conversation

@mark-sil

@mark-sil mark-sil commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs holds the three new answerers (jump, swap, convert) and the seams they run through.

The Avalonia detail view now answers the jump, swap and convert commands of the Lexeme Form row natively. Its label menu and the in-string right-click menus of the Lexeme Form, Citation Form and variant form rows build and run without the hidden WinForms DataTree or the mediator. Both views share the rules: DetailRules/DetailJumpTargets (the jump resolver, all four tool branches) and DetailRules/LexemeFormConversion (swap, convert and the data-loss warning). DataTree and LexEntryMenuHandler now call them.

What changes for WinForms users? Nothing visible. The WinForms handlers call the same rules and get the same answers. Two crash paths go away: Swap and Convert on an entry with no lexeme form now hide the item. The review worth your time is whether the native answers match what the adapter answered.

Where to look

  • Jump targets. DetailJumpTargets.Resolve reproduces the WinForms resolver, including its field rule: a header row has no field, and a row whose field is unknown declines a concordance jump. Pinned by DetailJumpTargetsTests and the adapter-equivalence test over every Lexicon row that binds an owned id.
  • Ownership. The predicate also switches over the two Texts Info jump menus, mnuDataTree-Text and mnuTextInfo-Notebook. LexiconMenuAuthorityTests.PinnedOwnedIds lists all 23 owned ids.
  • Mutations. Swap and convert are one undo step each, proved end to end in LexiconMenuExecutionTests.
  • Resources. SwapLexemeWithAllomorphDlg moved from LexEdDll to DetailControls so xWorks can show it. Its 3 strings and the 9 conversion warnings moved resx with the English unchanged.

Deliberately different: a jump with no target is disabled, where the adapter showed it enabled and inert. The other rules kept on purpose are under Decisions.

After merge: Crowdin stores translations by file path, so the moved dialog resx and the 12 moved strings arrive as new strings. They need to be approved in Crowdin.

Verification: build and both hygiene gates clean. Targeted tests pass: xWorksTests 88, FwAvaloniaTests 41, FdoUiTests 25, DetailControlsTests 16. A 7-step manual pass in Lexicon Edit, both views, passed. The full suite was not run.

Next: approve, or name a row where a native answer differs from WinForms.


Reading this a year from now -- start here

This is stage 2, PR B of LT-22691: retiring the hidden WinForms DataTree that the Avalonia detail view kept alive to answer context-menu commands. Each PR adds answerers to LexiconMenuAuthority, which owns a menu id once every leaf under it has an answerer; the ownership test pins the resulting list.

Until this PR the Avalonia view built the hidden tree to answer these commands, because a jump needed the current slice's object and field, and swap and convert lived in the WinForms menu handler.

Decisions, and why
  • A jump with no target is disabled. On the adapter path, when DataTree found no target, no colleague answered the display query and xCore's default showed the item enabled; clicking it did nothing. A native item cannot be inert, so it says so.
  • Swap and Convert act on the entry the row's object belongs to. WinForms used the tree's root entry. On the only rows that bind these ids the two are the same entry; the row is the identity the Avalonia host has.
  • Every native jump posts FollowLink through one host seam, RecordEditView.FollowLink. The chooser links and the anthropology filter route through it too, so when the mediator post is replaced (by a direct call or Pub/Sub) it changes in one place. The post stays deferred for now, like every other PostMessage("FollowLink").
  • After a conversion the re-show focuses the new form's row by object (DetailHostControl.FocusObjectOnNextShow). The new form has a new hvo, so its row ids are unknown to the outgoing view; PR A's focus-by-row-id seam cannot name them.
  • All four branches of the jump resolver moved, not only the Lexicon ones, because DataTree must call the shared rule either way. The notebook branch keeps its one side effect: a real jump from a text without a record creates and associates the record first; the rule reports that and the caller creates it.
  • The swap dialog is reused, not converted, as with ConfigureWritingSystemsDlg. It moved to DetailControls because xWorks cannot reference LexEdDll. The conversion warning is a confirmation seam on RecordEditView, like DeleteConfirmation.
  • DetailRules returns text, not UI. The data-loss warning text moved to FdoUiStrings so the shared rule can return it; the caller shows it.
Tests
  • DetailJumpTargetsTests (FdoUiTests): every branch of the jump rule, including the notebook record created for a text without one.
  • LexemeFormConversionTests (FdoUiTests): swap and convert as one undo step each, and every data-loss warning case.
  • LexiconMenuAuthorityTests: the 23-id pin, leaf coverage over every owned id, equivalence against the adapter path on all seven Lexicon rows binding an owned id, and the no-mediator spy.
  • LexiconMenuExecutionTests: swap (accepted and cancelled), convert (silent, warned and declined, warned and accepted), the in-string jumps, and the notebook jump creating its record once.
  • DetailFocusMemoryTests: the re-show focusing a row by object.
  • DetailObjectCommandExecutionTests: the ownership test now expects the Lexeme Form row's label menu to be fully owned.
Manual test steps (Lexicon Edit, Avalonia view unless stated)
  1. An entry with allomorphs: the Lexeme Form row's label menu offers Show Lexeme Form in Concordance and Swap Lexeme Form with Allomorph, and no Convert items. Swap opens the dialog; OK swaps; one Undo swaps back.
  2. An entry without allomorphs: Swap is absent.
  3. An affix entry: Convert to Affix Process only. With an environment on the form, the "environment will be lost" warning appears; No leaves it, Yes converts and focuses the new form's row, one Undo restores both. On the process, Convert to Affix Form warns that the rule will be lost.
  4. Right-click inside the Lexeme Form text: Show Entry in Concordance, Show Lexeme Form in Concordance, Swap. Inside Citation Form: Show Entry in Concordance.
  5. An entry with a variant: right-click inside the variant's Form text in the Variants section; Show Entry in Lexicon lands on the variant entry.
  6. Type in the Citation Form text, stay in the box, right-click there and choose Show Entry in Concordance: the concordance opens and the typed text is saved.
  7. WinForms view: swap, the convert warnings and the jumps behave as before.
Preflight review details

Code Review Summary

Branch: LT-22691o
Base: main (merge base 2b981ed, 1 commit ahead)
Date: 2026-10-14
Review model: Claude Opus 5.5 (Claude Code)
Files changed: 25

Overview

Stage 2, PR B of the hidden-DataTree menu-id retirement (LT-22691). The Lexicon menu
authority now answers JumpToTool, SwapLexemeWithAllomorph and ConvertLexemeForm
from the row, so the Lexeme Form row's label menu and the in-string menus of the Lexeme
Form, Citation Form and variant form rows build and run without the hidden WinForms
DataTree or the mediator. By the ownership predicate the two Texts Info jump menus
(mnuDataTree-Text, mnuTextInfo-Notebook) switch over too.

Both views share two new rules in Src/FdoUi/DetailRules: DetailJumpTargets (all four
branches of the WinForms jump resolver, which now calls it) and LexemeFormConversion
(swap, convert, data-loss warning; LexEntryMenuHandler calls it). The analysis found the
change complete for its purpose; the adapter-equivalence test passes on every Lexicon row
that binds an owned id.

Contract/API Changes

  • SwapLexemeWithAllomorphDlg moves from LexEdDll (SIL.FieldWorks.XWorks.LexEd) to
    DetailControls (SIL.FieldWorks.Common.Framework.DetailControls). Its only callers,
    LexEntryMenuHandler and the Avalonia host, are updated; its resources embed under the
    new name.
  • Twelve resx strings move with unchanged English: ksSwapLexWithAllo, ksAlternateForms,
    ksSwapXWithY from LexEdStrings to DetailControlsStrings; the nine ksConvertFormLose*
    warnings from LexEdStrings to FdoUiStrings.
  • DataTree.GetGuidForJumpToTool keeps its signature and results; its body delegates to
    the shared rule.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

  • Crowdin stores translations by source path, so the moved resx file and strings
    arrive as new strings
    (author: after merge the new strings need to be approved in
    Crowdin; the PR description says so, and the author will settle who approves them)

Minor - Consider

  • The host's notebook-record jump branch had no test through the host (fixed
    during review: NotebookJump_FromATextWithoutARecord_CreatesTheRecord_AndShowsIt)
  • CaptureFollowLink was duplicated in two fixtures (fixed during review: shared
    FollowLinkSpy.Capture)

Required Validation / Evidence

Positive Observations

  • The WinForms Swap and Convert no longer throw on an entry with no lexeme form; the
    shared rule checks it.
  • Every native jump posts FollowLink through one host seam, so replacing the mediator
    post later is a one-place change.
  • Execution tests prove each command end to end through the product host, including one
    Undo per mutation and the dialog and warning answers.

Interview Notes

  • Q1 (Crowdin): resolved as above.
  • Q2 walkthrough, in the author's words: "It was not obvious how these changes would
    affect localization. but it looks like Crowdin will automatically reuse the existing
    translations."
  • Q3: the author accepts the targeted tests as cover for the rebase; no manual re-run.
  • Q4: both Minor findings fixed during review, amended into the single commit at the
    author's request.
  • Q5: nothing else to flag.

In-Review Quality Check

The two test-only fixes were built with both hygiene gates and the full targeted set rerun
on the amended commit.

Suggested Review Focus

  • DetailJumpTargets.Resolve against the WinForms resolver it replaced, especially the
    field rule (no field for a header row, 0 for an unknown field).
  • The divergences kept on purpose: a targetless jump is disabled; Swap and Convert act
    on the row's entry.
  • The resource moves and the Crowdin approval note.

🤖 Generated with Claude Code


This change is Reviewable

The Lexicon menu authority now answers JumpToTool, SwapLexemeWithAllomorph
and ConvertLexemeForm from the row itself. The Lexeme Form row's label menu
and the in-string menus of the Lexeme Form, Citation Form and variant form
rows (plus the two Texts Info jump menus, owned by the same predicate) build
and run without the hidden WinForms DataTree or the mediator.

Both detail views share the rules: The WinForms DataTree and
LexEntryMenuHandler call the same code, so the two views cannot drift.

SwapLexemeWithAllomorphDlg moves to DetailControls, where xWorks can reach it,
and its strings and the conversion warnings move to the resources of the
projects that now own them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files      1 suites   10m 32s ⏱️
6 685 tests 6 600 ✅ 85 💤 0 ❌
6 694 runs  6 609 ✅ 85 💤 0 ❌

Results for commit 356ebdb.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.65543% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.68%. Comparing base (2b981ed) to head (356ebdb).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs 76.41% 13 Missing and 12 partials ⚠️
Src/LexText/Lexicon/LexEntryMenuHandler.cs 5.55% 15 Missing and 2 partials ⚠️
Src/FdoUi/DetailRules/LexemeFormConversion.cs 78.94% 6 Missing and 6 partials ⚠️
...trols/DetailControls/SwapLexemeWithAllomorphDlg.cs 0.00% 3 Missing ⚠️
Src/Common/Controls/DetailControls/DataTree.cs 66.66% 1 Missing and 1 partial ⚠️
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs 77.77% 1 Missing and 1 partial ⚠️
Src/FdoUi/DetailRules/DetailJumpTargets.cs 96.61% 0 Missing and 2 partials ⚠️
Src/Common/FwAvalonia/DetailHostControl.cs 0.00% 0 Missing and 1 partial ⚠️
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #1186       +/-   ##
=========================================
+ Coverage      0   39.68%   +39.68%     
=========================================
  Files         0     1543     +1543     
  Lines         0   354983   +354983     
  Branches      0    41203    +41203     
=========================================
+ Hits          0   140865   +140865     
- Misses        0   184747   +184747     
- Partials      0    29371    +29371     
Files with missing lines Coverage Δ
Src/Common/FwAvalonia/Detail/DataTree.cs 93.50% <100.00%> (ø)
...nia/Hosting/RecordEditView.ReferenceVectorMenus.cs 44.50% <100.00%> (ø)
Src/Common/FwAvalonia/DetailHostControl.cs 65.62% <0.00%> (ø)
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs 63.20% <80.00%> (ø)
Src/Common/Controls/DetailControls/DataTree.cs 47.29% <66.66%> (ø)
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs 71.90% <77.77%> (ø)
Src/FdoUi/DetailRules/DetailJumpTargets.cs 96.61% <96.61%> (ø)
...trols/DetailControls/SwapLexemeWithAllomorphDlg.cs 0.00% <0.00%> (ø)
Src/FdoUi/DetailRules/LexemeFormConversion.cs 78.94% <78.94%> (ø)
Src/LexText/Lexicon/LexEntryMenuHandler.cs 15.66% <5.55%> (ø)
... and 1 more

... and 1532 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thejambi thejambi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@thejambi partially reviewed 25 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on mark-sil).

@mark-sil
mark-sil merged commit b8926da into main Oct 15, 2026
9 checks passed
@mark-sil
mark-sil deleted the LT-22691o branch October 15, 2026 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants