Repository navigation
Conversation
The entry-search dialogs' results showed "***" for a sense with no gloss and "???" as the headword of an entry with no form, where the WinForms dialogs show a blank. The Glosses and Lexeme Form columns, the sense rows and the description text took LCM's best-alternative strings, which substitute a placeholder when nothing is stored. They now read the first stored value across the current writing systems through a new FirstText helper, and HeadwordText returns an empty string when no writing system has a form. An unglossed sense row falls back to the sense's chooser name, as intended. The Merge confirmation still names a formless entry "???", as the WinForms Merge dialog does, rather than leaving a blank in the sentence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## LT-22671-go-dialog-ws-selector #1218 +/- ##
==================================================================
- Coverage 40.38% 40.02% -0.36%
==================================================================
Files 1551 1545 -6
Lines 355742 355454 -288
Branches 41508 41392 -116
==================================================================
- Hits 143650 142262 -1388
- Misses 182770 183848 +1078
- Partials 29322 29344 +22
🚀 New features to boost your workflow:
|
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.
Start here:
Src/LexText/LexTextControls/Avalonia/EntryGoLauncherShared.cs--FirstTextand its callers are the fix;LcmMergeEntryDialogLauncher.ConfirmNameis the one exception.Stacked on #1217. Review only the top commit; this retargets to
mainonce #1217 merges.What it does
The Avalonia Go-family results lists (Go to Entry, Merge, Add/Link Allomorph, Link MSA, Link Entry or Sense, and Insert Entry's matching list) showed LCM's missing-value placeholders where the legacy lists show a blank:
***in Glosses for an unglossed sense, and???as the headword of an entry with no form. They now show blanks. The row's Lexeme Form value gets the same fix, though no dialog shows that column today."Why is there still a
????"A sense row with no gloss falls back to LCM's sense chooser label, as the legacy Link Entry or Sense sense list does; LCM writes
???there for a missing part of speech, in both UIs. The Merge confirmation also keeps???for a formless entry, as the legacy Merge dialog does, so the sentence never has a blank name. Everything else reads stored strings (StringOrNull) instead of LCM's display shortcuts (Best*Alternative,HomographForm).Where to look
FirstTextwriting-system order: analysis-first for glosses, vernacular-first for forms, as theBest*properties it replaces --GlossesText_*,LexemeFormText_NoFormInAnyWs_IsBlank.HeadwordTextreturns empty for a formless entry --HeadwordText_NoFormInAnyWs_IsBlank.ConfirmMessage_EntryWithNoForm_KeepsLcmsHeadwordPlaceholder.Not here
FirstTextskips LCM's last-resort user-interface writing system, so a value stored only there shows blank.???).Verification
LexTextControlsTests: 387 passed, 3 older skips; the 8 new tests each failed before the fix. Hygiene and gitlint clean. Manual checks, New UI matching legacy:...-- the unglossed sense row shows its chooser label, as legacy does.???.Next: review the top commit; approve, or name a list cell that should keep LCM's placeholder.
Reading this a year from now -- start here
LT-22883 was filed from findings during LT-22671 (#1217, the Go-dialog writing-system selector). There, an entry with no default-vernacular form showed
???as its headword; #1217 addedHeadwordText's fallback across writing systems, and the placeholders that remained --???for an entry with no form at all,***for unglossed senses, already onmain-- became this ticket. The record of why lists show blanks while the Merge sentence keeps???lives here and indialog-conversion.md§6; no working notes were committed.Decisions, and why
***/???. The placeholders come from LCM's display properties; filtering their text would also hide a stored value that happens to be those characters.FirstTextwalks the current analysis and vernacular writing systems withStringOrNulland returns the first non-empty value.???. A blank leaves a gap in "X will be merged into Y". The legacyMergeEntryDlgnames entries byILexEntry.HeadWord, which is???for a formless entry, soConfirmNamefalls back toHeadWordonly whenHeadwordTextis empty.ChooserNameTS. That fallback was always inSenseGloss;BestAnalysisAlternativereturned***and never let it fire. The chooser label is what the legacy sense list shows, including LCM's???for an empty part-of-speech slot.Paths not taken
Best*Alternative: rejected for the false-positive reason above.Evidence
New tests (
LexTextControlsTests):EntryGoDialogLauncherTests:GlossesText_UnglossedSense_IsBlank,GlossesText_UnglossedSenseAmongGlossedOnes_IsSkipped,SharedSearch_UnglossedSense_DescriptionShowsNoPlaceholder,SharedEntryOrSenseSearch_UnglossedSense_RowFallsBackToChooserName,LexemeFormText_NoFormInAnyWs_IsBlank,HeadwordText_NoFormInAnyWs_IsBlank.LcmMergeEntryDialogLauncherTests:ConfirmMessage_NamesBothEntriesByHeadword,ConfirmMessage_EntryWithNoForm_KeepsLcmsHeadwordPlaceholder.HeadwordTextcallers, all checked: the entry and sense result rows and their description (EntryGoLauncherShared), Insert Entry's matching list (LcmInsertEntryDialogLauncher), and the Merge confirmation (LcmMergeEntryDialogLauncher, which substitutesHeadWord).The row description text is not shown by any product dialog today (no launcher sets
DescriptionLabel), so only the unit test covers it.Preflight review details
Code Review Summary
Branch: LT-22883-go-results-placeholders
Base: LT-22671-go-dialog-ws-selector (stacked on #1217; retarget to main after #1217 merges)
Date: 2026-10-30
Review model: Claude Opus 5.5
Files changed: 5 (1 commit) vs the stacked base; 20 files / 4 commits vs origin/main
Overview
LT-22883: the Avalonia Go-family results list (Go to Entry, Merge, Add/Link Allomorph,
Link MSA, Link Entry or Sense, plus Insert Entry's matching list) showed LCM's missing-value
placeholders --
***fromBest*Alternativefor an unglossed sense or an empty lexemeform, and
???fromHomographFormfor an entry with no form -- where the WinForms listsshow a blank. A new
EntryGoLauncherShared.FirstTextreturns the first stored value acrossthe current writing systems, or empty; the Glosses, Lexeme Form and sense-row text use it,
and
HeadwordTextnow returns empty. An unglossed sense row therefore falls back to LCM'ssense chooser label, as the code always intended and as the WinForms sense list shows.
The Merge confirmation is the one deliberate exception: a sentence with a blank name reads
badly, and the WinForms
MergeEntryDlgnames a formless entry byHeadWord(???), soConfirmMessagekeeps LCM's headword for that case (full parity, author's choice).Contract/API Changes
EntryGoLauncherShared.HeadwordTextreturns""(wasHomographForm, i.e.???) for anentry with no form in any current writing system. Internal; callers are the results rows,
Insert Entry's matching list, and Merge's confirmation (which now substitutes
HeadWord).EntryGoLauncherShared.FirstTextandLcmMergeEntryDialogLauncher.ConfirmMessage.Findings
Critical - Must address before merge
None.
Important - Should address before merge
None.
Minor - Consider
A formless entry's results row now has an empty accessible name ((accepted: rare data, WinForms browse rows expose no better name; not worth a change in FwAvaloniaDialogs here)EntryGoDialogView.axaml.cs:255usesrow.Text; was???)(accepted: matches the Glosses column, which already used analysis-then-vernacular; a vernacular-ws gloss is unusual)SenseGloss/DescriptionTextnow try vernacular writing systems before the chooser-label fallback (wasBestAnalysisAlternative, analysis only)(accepted: lists show current writing systems, as WinForms columns do)FirstTextdrops LCM's last-resort user-interface-writing-system fallback, so a value stored only there shows blankRequired Validation / Evidence
.\test.ps1 -TestProject LexTextControlsTests -CommentHygiene -TokenHygiene: 390 total,387 passed, 3 skipped (pre-existing skips). The 8 new tests failed before the fix.
-BaseRef origin/main) and token hygiene clean; gitlint onorigin/LT-22671-go-dialog-ws-selector..HEADexit 0.second, not***, second).WinForms record list.
chooser label
1 ??? a test definition-- the???is the empty part-of-speech slot,identical in the WinForms sense list.
???, matching WinForms.pane), covered by
SharedSearch_UnglossedSense_DescriptionShowsNoPlaceholder; and therow's Lexeme Form value (every launcher uses
BuildDefaultResultColumns, Headword andGlosses only), covered by
LexemeFormText_NoFormInAnyWs_IsBlank.Positive Observations
dialog-conversion.md§6 so the next Avalonia list avoids it.Interview Notes
findings during LT-22671 / LT-22671: Add a writing-system selector to the Go-family dialogs #1217).
???in the sentence while lists showblanks.
Lexeme Form); create it in the WinForms UI. Author had nothing further to flag.
Suggested Review Focus
FirstTextwriting-system order (analysis-first for glosses, vernacular-first for forms)LcmMergeEntryDialogLauncher.ConfirmName🤖 Generated with Claude Code
This change is