Repository navigation
Conversation
InterlinMaster now moves its tab control itself whenever it sets the InterlinearTab property: the tab click, the getter's normalizing fallback and the Baseline tab forced for an empty text all store the value without a broadcast, and the forced set calls ShowTabView directly when the same text is shown again. Until now each of those sets queued an OnPropertyChanged broadcast whose only work was that one ShowTabView, run after the tab control already matched the property or, for the forced set, after the rest of ShowRecord had run with the analysis tab still selected. The OnPropertyChanged handler stays for the one sender left: a followed link that carries the tab. Property whose own sets stop broadcasting: * InterlinearTab Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1216 +/- ##
==========================================
+ Coverage 40.36% 40.40% +0.04%
==========================================
Files 1550 1550
Lines 355615 355621 +6
Branches 41476 41477 +1
==========================================
+ Hits 143536 143688 +152
+ Misses 182770 182640 -130
+ Partials 29309 29293 -16
🚀 New features to boost your workflow:
|
thejambi
approved these changes
Oct 29, 2026
thejambi
left a comment
Contributor
There was a problem hiding this comment.
@thejambi reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on mark-sil).
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/Interlinear/InterlinMaster.cs,ShowRecord— thefForcedRawTextflag and the directShowTabView()in the same-text branch. The setter change near the end of the file is one flag.InterlinMaster's own sets of the
InterlinearTabproperty — the tab click, the getter's normalizing fallback, and the Baseline tab forced for an empty text — no longer broadcastOnPropertyChanged. The view moves its tab control itself. The handler stays for the one sender left: a followed link that carries the tab (Go Back / Go Forward,silfw:URLs).Does anything still need the queued
ShowTabView? Two of the three sets never did: after a click the tab control already shows the clicked tab, and the fallback fires fromShowTabView's own read or from readers that run after it, so the index already matches when the broadcast drains. The forced set did one piece of work: when an empty text is shown again on an analysis tab (F5), nothing else inShowRecordmoves the tab control, and the queued handler switched it to Baseline afterwards. That call is now made directly in that branch. Same end state; the switch happens insideShowRecordinstead of at the next Mediator drain, so the analysis tab is no longer selected and focused in between.Where to look
ShowRecord(same text): the direct call follows today'sSelectAnnotation, so every call today's path makes still runs, in order. Pinned byShowRecord_EmptyTextOnAnalysisTab_MovesToRawTextWithoutBroadcast.LinkListener.FollowActiveLink; checked manually with Go Back.ShowTabViewinsideShowRecordis already the case throughSwitchText, so no new re-entry context.Deliberately not here: the link sender and its broadcast (the follow-link workflow). The fallback's
""value is no longer created by reads since #1213.Verification:
build.ps1 -CommentHygiene -TokenHygieneclean; ITextDllTestsInterlinMasterTabTests,InterlinMasterTests,RawTextPaneTests8/8 (4 new; the tab-click and ShowRecord tests fail on main). Manual on Sena 3: tab clicks with the pane bar's "Add Words to Lexicon" following the tab; Go Back restoring Gloss on the same text; restart restoring Analyze; Parser menu items following the tab; an empty text on Gloss moved to Baseline by F5. Not run: the full ITextDllTests suite.Next: approve.
Reading this a year from now -- start here
One of the small PRs converting
PropertyTablebroadcasts (OnPropertyChangedthrough the xCore Mediator) to direct calls ahead of the Pub/Sub delivery design. The inventory that classifies every broadcasting site is a gitignored working document, so the reasoning for this site lives here.Decisions, and why
ShowTabViewrather than keeping the broadcast flag on the forced set. The forced set's row cited only re-entry (the handler callsShowTabView, which reads the property), which the conversion's step order resolves with a direct call. Keeping the flag would have left one broadcasting self-set and the row in the inventory.else if (m_tabCtrl.SelectedIndex != (int)InterlinearTab)would also catch a link-set tab on the text already shown; that path belongs to the follow-link workflow and keeps its queued handler.SelectAnnotationkept before the direct call. It runs on the analysis pane (for an empty text its bookmark converts to no occurrence andSelectOccurrence(null)hides the focus box) andShowTabViewruns it again on the Baseline pane — the same two calls today's path makes, in the same order. Dropping the first would be the one real behaviour change.LinkListener.FollowActiveLink, which sets every property entry a link carries with a broadcast; the entries come fromUpdateContextHistory(history links) and from "Copy Location as Hyperlink" URLs. It does work only for a link to the text already shown with a different tab; other links go throughSwitchTextor a new view'sInit, which read the property before the drain.Evidence
InterlinMaster.csandLinkListener.cs:584;ParserListener.cs:1059only reads it.InterlinMaster.OnPropertyChangedis the onlyInterlinearTabhandler; it callsShowTabViewonly whenm_tabCtrl.SelectedIndexdiffers from the property.FwXWindow.OnMasterRefresh→FinishRefresh→RefreshDisplay→clerk.ReloadIfNeeded→ReloadList→OnListChanged(Normal)→BroadcastChange→ RecordNavigation →RecordView.ShowRecord(rni)→InterlinMaster.ShowRecord, withRootStTextunchanged.TabControlwith a created handle raisesDeselecting/Selectedon a programmaticSelectedIndexchange (set_SelectedIndex→WmSelChange→OnSelected), soClickTabexercises the real click handler; before the handle exists the change is silent, whichStartOnTabrelies on.InterlinMaster(orInterlinMasterNoTitleBar) per tool in the Texts and Concordance configurations, so one view is a target at a time.Preflight review details
Code Review Summary
Branch: claude/pubsub-interlinear-tab-1d1843
Base: main (merge base 936ae91)
Date: 2026-10-29
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 3
Overview
InterlinMaster's own sets of the
InterlinearTabproperty (the tab click, the getter's normalizing fallback, the Baseline tab forced for an empty text) no longer broadcastOnPropertyChanged; the view moves its tab control itself. The forced set callsShowTabViewdirectly in the branch where the same text is shown again, which is the only path where the queued handler did work. The handler stays for the one sender left, a followed link that carries the tab (LinkListener.FollowActiveLink).Analysis (all four passes run directly; small managed diff, no native, COM, localization, build or installer surface): no Critical or Important finding. The change is a reordering with the same end state: the tab switch happens inside
ShowRecordinstead of at the next Mediator drain.Contract/API Changes
None.
InterlinearTabisinternal; its setter stops broadcasting. TheRawTextPaneTests.ListeningColleaguetest helper goes fromprivatetointernalso the new fixture can reuse it.Findings
Critical - Must address before merge
None.
Important - Should address before merge
None.
Minor - Consider
(deliberate: keeps today's call sequence exactly, moving only the ShowTabView; author chose this shape over dropping the call)SelectAnnotation()runs on the analysis pane right before the forcedShowTabView(), which calls it again on the Baseline paneListeningColleagueduplicated from RawTextPaneTests (fixed during review: the new fixture reusesRawTextPaneTests.ListeningColleague, widened to internal)m_tabCtrl.Handleaccess afterCreateControl()in the test fixture (fixed during review: removed)Required Validation / Evidence
.\build.ps1 -CommentHygiene -TokenHygiene: build succeeded, 0 warnings, both hygiene gates clean..\test.ps1 -CommentHygiene -TokenHygiene -TestProject ITextDllTests -TestFilter "FullyQualifiedName~InterlinMaster|FullyQualifiedName~RawTextPaneTests": 8/8 passed (4 newInterlinMasterTabTests, 2InterlinMasterTests, 2RawTextPaneTests). The two tab tests and the ShowRecord test fail on main (JobItems 1; the tab control left on Gloss).Positive Observations
ShowTabViewalready runs insideShowRecord(theSwitchTextbranch), so no new re-entry context.Interview Notes
ShowTabView(option B) over keeping the broadcast flag (option A) after reviewing the timing difference: the analysis tab is no longer selected and focused for the span betweenShowRecordand the Mediator drain; end state identical.SelectAnnotationbefore the direct call is kept on purpose so every call today's path makes still happens in today's order.ListeningColleaguestays in RawTextPaneTests (author declined a separate file).In-Review Quality Check
Build and the three fixtures re-run after the two test-fixture cleanups: 8/8 passed, hygiene clean.
Suggested Review Focus
InterlinMaster.ShowRecord: thefForcedRawTextflag and the directShowTabView()in the same-text branch.InterlinMasterTabTests.ShowRecord_EmptyTextOnAnalysisTab_MovesToRawTextWithoutBroadcastas the pin for that branch.🤖 Generated with Claude Code
This change is