LT-22671: Add a writing-system selector to the Go-family dialogs by thejambi · Pull Request #1217 · sillsdev/FieldWorks

thejambi · 2026-10-29T22:39:50Z

Start here: Src/LexText/LexTextControls/Avalonia/EntryGoLauncherShared.cs -- SearchWritingSystem, GetSearchFields and BuildSearchFieldSpec are the mechanism; the rest plumbs into them.

Every Avalonia Go-family dialog (Go to Entry, Merge Entry, Add Allomorph, Link Allomorph, Link MSA, Link Entry or Sense) gets the BaseGoDlg "Writing System:" selector. An analysis writing system searches sense glosses; a vernacular one searches citation, lexeme and alternate forms. The search box follows the choice's font, direction and keyboard.

Headless captures (Avalonia.Headless, test data): Spanish selected; Arabic selected, the search box flips right-to-left.

Is this only a dropdown? No: the legacy selector changes which fields are searched. The dialog stays LCModel-free (options and a WritingSystemChanged callback on EntryGoSearchFieldSpec); each launcher shares one SearchWritingSystem between that callback and its search delegate, which reads it on every query. The question worth your time is whether that hand-off is ordered correctly.

Where to look

EntryGoDialogViewModel.OnSelectedWritingSystemChanged: the callback must run before RunSearch. Pinned by SharedSearch_SelectionChangedAfterBuild_NextSearchUsesNewWs and GoFamilyBuildInputs_SelectorDrivesTheirSearch (all six dialogs).
GetSearchFields: analysis to glosses, vernacular to forms, both to both.
EntryGoDialogView.ApplySearchTypography: clears what the new writing system leaves unset, so right-to-left reverts.
HeadwordText falls back across writing systems instead of showing "???"; Insert Entry and Merge use it too.

Not here

Results-list display gaps already on main: *** for an empty gloss (LT-22883) and the headword font per value (LT-22884).
Deliberate differences: focus stays on the picker after a switch; alternate forms are always searched; the picker sits in a framed host, as Insert Entry's do.

Verification: test.ps1 FwAvaloniaDialogsTests 297/297; LexTextControlsTests 379 passed, 3 pre-existing skips; comment and token hygiene clean. Author ran Go to Entry, Merge, Link Entry or Sense and Add Allomorph under the new UI, and re-checked Add Allomorph after the review fixes. Not run: a full build.ps1. Branched from 380cc90; main has moved on, no overlapping files.

Next: check the hand-off ordering and approve, or name a deliberate difference to change.

Reading this a year from now -- start here

This PR closes GAP-WS-SELECTOR from the control exemplar map. The durable design note is dialog-conversion.md section 2d (in the tree); the exemplar map row and a lessons-ledger entry were added alongside it. Nothing was deleted from the branch: its only markdown changes are those three skill references. The reasoning below is what the code and docs do not say.

The layer cake -- from a pick to a search

EntryGoDialogView builds a dropdown FwOptionChooser into PART_WritingSystemHost from EntryGoDialogViewModel.WritingSystems. Committing an option sets SelectedWritingSystem.
EntryGoDialogViewModel.OnSelectedWritingSystemChanged invokes EntryGoSearchFieldSpec.WritingSystemChanged(id), raises SearchTypography, then re-runs the search.
The launcher's callback (from EntryGoLauncherShared.BuildSearchFieldSpec) points the shared SearchWritingSystem.Handle at the chosen writing system.
The search delegate (BuildEntrySearch / BuildEntryOrSenseSearch) reads the handle and asks GetSearchFields for fields: vernacular gives citation, lexeme and alternate forms; analysis gives sense glosses.
EntryGoSearchEngine runs those fields; rows take their headword from HeadwordText.
The view re-applies font, size and direction from SearchTypography, clearing whatever the new writing system leaves unset.

Decisions, and why

A shared mutable holder instead of a writing-system parameter on the search delegate. Changing Func<string, ...> to carry the writing system would have touched EntryGoDialogInput, the ViewModel and every launcher's search contract. The holder confines the change to the launcher layer. Cost: the ViewModel must report the change before it searches.
FwOptionChooser, not a ComboBox. The exemplar map's dialog rule forbids ad-hoc ComboBoxes.
Picker built in the view's code-behind, not the ViewModel. Insert Entry builds its pickers in its ViewModel; keeping this ViewModel control-free lets its tests run without a UI.
Analysis searches glosses only. EntryGoDlg.GetFields also searches definitions when the Definitions column is visible; it is hidden by default and the Avalonia list has no column chooser, so the default is what ships.
Add Allomorph starts on the form's writing system (BaseGoDlg.SetDlgInfo(..., tssform)). Go to Entry, Merge and Link Entry or Sense start on the default vernacular one in WinForms too. A starting writing system that is not current falls back to the default vernacular one, so the selection and the search cannot disagree.
Options are sorted by display name, as the WinForms combo (Sorted = true) is, not grouped by kind.

Paths not taken

A plain host border without fwFieldHost, to avoid the doubled frame: the view harness fails any PART_*Host with a zero border, and all 45 view tests failed. The framed host is the convention.
Refocusing the search box after a switch, as BaseGoDlg does: arrow keys on a collapsed picker change the selection, so stealing focus breaks browsing. The keyboard switch happens when the user returns to the box.

Surprising findings

The gap note framed the selector as typography and keyboard only. The WinForms change handler also changes the searched fields, which is most of the work here.
Manual testing showed "???" for an entry whose only form is in a second vernacular writing system. LCM's HeadWord reads the default vernacular writing system only; the WinForms browser column uses best vernoranal. The bug predates this branch but was unreachable until the selector let a search find such entries.

Deferred, and what would unblock it

Headword column font per value (LT-22884). The column renders in the default vernacular font even when the headword came from another writing system. Needs per-row typography on EntryGoResultColumn rather than per-column.
LCM placeholders in the results list (LT-22883). GlossesText, SenseGloss and LexemeFormText use LCM's best-alternative properties, which return *** when nothing is stored, and HeadwordText's last fallback returns ??? for an entry with no form at all. Already on main; found while manually testing this PR.
Restricted option lists. WordformGoDlg (vernacular only) and ReversalEntryGoDlg (the index's writing system only) narrow the list. Neither is in the Avalonia family yet; porting one needs a way for the launcher to pass a subset to BuildSearchFieldSpec.

Evidence

New tests:

EntryGoDialogLauncherTests (real cache): SharedSearch_AnalysisWsSelected_FindsEntryByGloss, SharedSearch_VernacularWsSelected_DoesNotMatchGlosses, SharedSearch_SecondVernacularWsSelected_FindsEntryByFormInThatWs, SharedSearch_SelectionChangedAfterBuild_NextSearchUsesNewWs, SharedEntryOrSenseSearch_AnalysisWsSelected_SenseModeFindsSenseByGloss, BuildSearchFieldSpec_* (options, typography, callback, null argument), GoFamilyBuildInputs_SelectorDrivesTheirSearch, AddAllomorph_BuildInput_OpensOnTheFormsWritingSystem, AddAllomorph_BuildInput_FormInANonCurrentWs_OpensOnTheDefaultVernacular, HeadwordText_*.
EntryGoDialogTests (ViewModel and realized headless view): WritingSystemSelector_* -- initial selection, unknown-id fallback, change reports and re-searches, typography change, cleared selection, labelled picker, right-to-left and back, reset to defaults, hidden row.

Manual check, new UI:

Set $env:FW_AVALONIA = '1' and start Output\Debug\FieldWorks.exe; choose the new UI in Tools > Options > General.
In Lexicon Edit press Ctrl+F; switch the Writing System picker to an analysis writing system and type part of a gloss.
In Lexicon > Reversal Indexes, use a reversal entry's Senses chooser for Link Entry or Sense.
In a text whose baseline is a second vernacular writing system, use Lex. Entries > "Allomorph of..." in the Interlinear sandbox: the dialog should open on that writing system with no "???" rows.

Preflight review details

Code Review Summary

Branch: LT-22671-go-dialog-ws-selector
Base: main (merge base 380cc90; origin/main is 8 commits ahead, none touching these files)
Date: 2026-10-29
Review model: Claude Opus 5.5
Files changed: 19 (3 commits: feature, in-review fixes, doc alignment)

Overview

LT-22671 adds the WinForms BaseGoDlg writing-system selector to every Avalonia
entry-search ("Go") dialog: Go to Entry, Merge Entry, Add Allomorph, Link
Allomorph, Link MSA, and Link Entry or Sense. A "Writing System:" row under the
search box offers every current analysis and vernacular writing system. Picking
one re-runs the search in it: analysis searches sense glosses, vernacular searches
citation, lexeme and alternate forms. The search box takes the chosen writing
system's font and direction and switches keyboard on focus.

The dialog layer stays LCModel-free: EntryGoSearchFieldSpec carries the options
and a WritingSystemChanged callback; each launcher shares one
EntryGoLauncherShared.SearchWritingSystem between that callback and its search
delegate, which reads it on every query. Analysis found one parity gap in Add
Allomorph, and manual testing found a "???" headword exposed by the new selector;
both were fixed during review.

Contract/API Changes

FwAvaloniaDialogs (public, additive): new EntryGoWritingSystemOption;
EntryGoSearchFieldSpec.WritingSystems, SelectedWritingSystemId,
WritingSystemChanged; EntryGoDialogViewModel.WritingSystems,
HasWritingSystemSelector, WritingSystemLabel, SelectedWritingSystem,
SearchTypography.
New resource key FwAvaloniaDialogs.EntryGoWritingSystemLabel ("Writing System:",
same seed text as LexTextControls.ks_WritingSystem_).
Internal only: BuildVernacularSearchFieldSpec(cache) replaced by
BuildSearchFieldSpec(cache, searchWs); GetSearchFields takes the cache;
BuildEntrySearch / BuildEntryOrSenseSearch and the six launchers'
BuildSearch gain an optional SearchWritingSystem. All callers updated.
Behavior: EntryGoLauncherShared.HeadwordText now falls back across current
writing systems; it also feeds Insert Entry's matching list and Merge's
confirmation text.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

Add Allomorph opened on the default vernacular writing system instead of
the form's (fixed during review: LcmAddAllomorphDialogLauncher.BuildInput
seeds the shared writing system from tssForm; BuildSearchFieldSpec replaces a
non-current one with the default vernacular. Tests
AddAllomorph_BuildInput_OpensOnTheFormsWritingSystem,
AddAllomorph_BuildInput_FormInANonCurrentWs_OpensOnTheDefaultVernacular.)
Result rows showed "???" for entries with no default-vernacular form
(found in manual testing; fixed during review: HeadwordText tries each current
vernacular writing system, default first, then analysis, matching the WinForms
column's best vernoranal. Tests
HeadwordText_NoDefaultVernacularForm_UsesAnotherVernacularForm,
HeadwordText_DefaultVernacularForm_IsPreferred.)

Minor - Consider

Headword column renders in the default vernacular font even when the
headword comes from another writing system (author: leave for a follow-up; filed as LT-22884)

Required Validation / Evidence

Run:

./test.ps1 -TestProject Src/LexText/LexTextControls/LexTextControlsTests -CommentHygiene -TokenHygiene after the in-review fixes: 379 passed, 3 skipped
(pre-existing: two LIFT import tests, GlossesBeforeWords; plus the explicit
screenshot harness), 0 failed.
./test.ps1 -TestProject Src/Common/FwAvaloniaDialogs/FwAvaloniaDialogsTests -CommentHygiene -TokenHygiene: 297/297 (the in-review fixes do not touch this
project).
Build/Agent/comment-hygiene.ps1 and token-hygiene.ps1: clean.
gitlint on the branch commits: clean.

Manual (author-confirmed): Go to Entry, Merge Entry, and Link Entry or Sense (via
Reversal Indexes > Senses) under the new UI with FW_AVALONIA=1; the Add
Allomorph repro in the Interlinear sandbox, and its re-check after the in-review
fixes (Spanish preselected, "chucho" listed, no "???").

Still needed:

./build.ps1 full build was not run by the reviewer; test.ps1 built the touched
projects.

Positive Observations

Opt-in shape: a spec with no options keeps every dialog unchanged, and the
existing EntryGo tests pass untouched.
Tests at all three layers: launcher over a real cache (search per writing system,
selection after build, all six dialogs), ViewModel (selection, fallback, cleared
selection, typography change), and realized headless view (picker, RTL and back,
reset to defaults, hidden row).
Uses the house FwOptionChooser rather than an ad-hoc ComboBox, per the control
exemplar map.
The new string resolves from the neutral resx
(EveryAccessorProperty_ResolvesFromTheNeutralResx).
Closes GAP-WS-SELECTOR in the exemplar map, documents the design in
dialog-conversion.md 2d, and records the scope lesson in lessons-learned.md.

Interview Notes

Jira: user-visible change, ticket LT-22671 present.
Deliberate choices (author confirmed): the picker shows a double frame
(the harness requires a framed PART_*Host, and the dropdown draws its own);
focus stays on the picker after a switch (the WinForms dialog refocused the
search box, which would break arrow-key browsing of the collapsed picker);
alternate forms are always searched for a vernacular writing system (the
WinForms dialog searched them only when the Allomorphs column was visible).
Author does not understand: how the shared SearchWritingSystem connects the
picker's selection callback to the search delegate, and why the ViewModel must
invoke WritingSystemChanged before it re-runs the search. Author answered
"not sure".
No other trade-offs or context flagged by the author.

In-Review Quality Check

The two Important fixes are the second commit. After them the full
LexTextControlsTests run passed with clean comment and token hygiene, and no
comment lines were re-wrapped. A third commit aligns dialog-conversion.md 2d
with the fixes (doc only).

Suggested Review Focus

Ordering in EntryGoDialogViewModel.OnSelectedWritingSystemChanged: the
callback updates the shared SearchWritingSystem before RunSearch reads it
(the area the author is unsure of).
EntryGoLauncherShared.GetSearchFields: analysis to glosses, vernacular to
forms, both to both, neither to forms.
EntryGoDialogView.ApplySearchTypography: clears values the new writing
system leaves unset.
HeadwordText fallback and its effect on Insert Entry and Merge text.

🤖 Generated with Claude Code

This change is

Every entry-search dialog (Go to Entry, Merge Entry, Add Allomorph, Link Allomorph, Link MSA, Link Entry or Sense) now shows a "Writing System:" picker under the search box, as the WinForms BaseGoDlg does. It lists each current analysis and vernacular writing system once, sorted by name, with the default vernacular one selected. Picking a writing system re-runs the search in it: an analysis writing system matches sense glosses, a vernacular one matches citation, lexeme and alternate forms. The search box takes the chosen writing system's font and direction, resetting them when the choice changes, and its keyboard switches when the box gains focus. The dialog stays free of LCModel: the search-box spec carries the options and a selection callback, and each launcher shares one SearchWritingSystem between that callback and its search, which reads it on every query. Closes GAP-WS-SELECTOR in the control exemplar map and documents the design in dialog-conversion.md 2d. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Add Allomorph now opens with the writing system of the form being added selected, as the WinForms dialog does, so a form typed in a second vernacular writing system is found straight away. A writing system that is not current falls back to the default vernacular one. Result rows no longer show "???" for an entry with no form in the default vernacular writing system: the headword falls back to the first current vernacular writing system with a form, then the analysis ones. The new selector made such entries reachable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The 2d note said the default vernacular writing system is always selected. It now describes the starting writing system the launcher supplies, Add Allomorph starting on the form's writing system, and the headword fallback across writing systems. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-29T22:56:31Z

NUnit Tests

1 files ± 0 1 suites ±0 15m 6s ⏱️ -1s
6 971 tests +23 6 886 ✅ +23 85 💤 ±0 0 ❌ ±0
6 980 runs +23 6 895 ✅ +23 85 💤 ±0 0 ❌ ±0

Results for commit ad464aa. ± Comparison against base commit 936ae91.

This pull request removes 1 and adds 24 tests. Note that renamed tests count towards both.

LexTextControlsTests.EntryGoDialogLauncherTests ‑ BuildVernacularSearchFieldSpec_CarriesTheDefaultVernacularTypography

FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_Change_RaisesSearchTypographyChange
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_Change_ReportsIdAndReRunsSearch
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_Cleared_KeepsSearchingInTheLastWritingSystem
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_NoOptions_IsHidden
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_OptionsAndInitialSelection_ComeFromTheSpec
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_UnknownInitialId_SelectsFirstAndReportsIt
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_View_HiddenWithoutOptions
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_View_ShowsLabelledPickerWithOptions
FwAvaloniaDialogsTests.EntryGoDialogTests ‑ WritingSystemSelector_View_SwitchingAppliesAndResetsTypography
LexTextControlsTests.EntryGoDialogLauncherTests ‑ AddAllomorph_BuildInput_FormInANonCurrentWs_OpensOnTheDefaultVernacular
…

codecov-commenter · 2026-10-29T22:57:21Z

Codecov Report

❌ Patch coverage is 84.21053% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.38%. Comparing base (380cc90) to head (ad464aa).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...ommon/FwAvaloniaDialogs/EntryGoDialogView.axaml.cs	66.00%	8 Missing and 9 partials ⚠️
.../LexTextControls/Avalonia/EntryGoLauncherShared.cs	90.56%	0 Missing and 5 partials ⚠️
...Common/FwAvaloniaDialogs/EntryGoDialogViewModel.cs	85.71%	0 Missing and 3 partials ⚠️
...on/FwAvaloniaDialogs/EntryGoWritingSystemOption.cs	0.00%	0 Missing and 1 partial ⚠️
...Controls/Avalonia/LcmAddAllomorphDialogLauncher.cs	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1217      +/-   ##
==========================================
+ Coverage   40.00%   40.38%   +0.37%     
==========================================
  Files        1544     1551       +7     
  Lines      355308   355742     +434     
  Branches    41357    41508     +151     
==========================================
+ Hits       142142   143650    +1508     
+ Misses     183833   182770    -1063     
+ Partials    29333    29322      -11

Files with missing lines	Coverage Δ
...c/Common/FwAvaloniaDialogs/EntryGoDialogView.axaml	`100.00% <100.00%> (ø)`
...mmon/FwAvaloniaDialogs/FwAvaloniaDialogsStrings.cs	`99.17% <100.00%> (+<0.01%)`	⬆️
...extControls/Avalonia/LcmGoToEntryDialogLauncher.cs	`36.53% <100.00%> (+1.24%)`	⬆️
...ontrols/Avalonia/LcmLinkAllomorphDialogLauncher.cs	`59.79% <100.00%> (+0.41%)`	⬆️
...rols/Avalonia/LcmLinkEntryOrSenseDialogLauncher.cs	`43.58% <100.00%> (+0.73%)`	⬆️
...xTextControls/Avalonia/LcmLinkMsaDialogLauncher.cs	`45.45% <100.00%> (+0.71%)`	⬆️
...xtControls/Avalonia/LcmMergeEntryDialogLauncher.cs	`32.22% <100.00%> (+0.76%)`	⬆️
...on/FwAvaloniaDialogs/EntryGoWritingSystemOption.cs	`0.00% <0.00%> (ø)`
...Controls/Avalonia/LcmAddAllomorphDialogLauncher.cs	`53.06% <87.50%> (+0.61%)`	⬆️
...Common/FwAvaloniaDialogs/EntryGoDialogViewModel.cs	`81.16% <85.71%> (+0.71%)`	⬆️
... and 2 more

... and 61 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.

Zachary Burnham and others added 3 commits October 29, 2026 15:40

thejambi marked this pull request as ready for review October 30, 2026 12:54

thejambi mentioned this pull request Oct 30, 2026

LT-22883: Show blanks, not LCM placeholders, in Go-dialog results #1218

Draft

6 tasks

thejambi added this pull request to stack #1219 October 30, 2026 19:43

Navigation Menu

Repository navigation

LT-22671: Add a writing-system selector to the Go-family dialogs - #1217

LT-22671: Add a writing-system selector to the Go-family dialogs#1217
thejambi wants to merge 3 commits into
mainfrom
LT-22671-go-dialog-ws-selector

thejambi commented Oct 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 29, 2026

Uh oh!

codecov-commenter commented Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

thejambi commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Overview

Contract/API Changes

Findings

Critical - Must address before merge

Important - Should address before merge

Minor - Consider

Required Validation / Evidence

Positive Observations

Interview Notes

In-Review Quality Check

Suggested Review Focus

Uh oh!

github-actions Bot commented Oct 29, 2026

NUnit Tests

Uh oh!

codecov-commenter commented Oct 29, 2026

Codecov Report

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thejambi commented Oct 29, 2026 •

edited

Loading