Skip to content

Remove dead FsFeatDefnUi class and stale JumpToRecord comments - #1045

Open
mark-sil wants to merge 1 commit into
mainfrom
remove-fsfeatdefnui-dead-code
Open

Remove dead FsFeatDefnUi class and stale JumpToRecord comments#1045
mark-sil wants to merge 1 commit into
mainfrom
remove-fsfeatdefnui-dead-code

Conversation

@mark-sil

@mark-sil mark-sil commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Dead-code removal ahead of the JumpToRecord Mediator-to-PubSub conversion (follow-up in the spirit of #1032/#1044). Removes one of the seven synchronous SendMessage("JumpToRecord") senders as dead code before the conversion starts.

  • Delete Src/FdoUi/FsFeatDefnUi.cs — the entire class is unreachable (see analysis below), including its SendMessage("JumpToRecord").
  • FdoUiCore.cs — remove the unreachable case FsFeatDefnTags.kClassId: from CmObjectUi.CreateNewUiObject.
  • MasterPhonFeatDlgListener.cs / MasterInflFeatDlgListener.cs — delete commented-out //m_mediator.SendMessage("JumpToRecord", ...) lines and fold the comment lines that referred to them into a single accurate line.

Why FsFeatDefnUi is unreachable

  • Its only external reference is the FsFeatDefnTags.kClassId case in CmObjectUi.CreateNewUiObject (FdoUiCore.cs).
  • The only call chain into that switch is DTMenuHandlerSlice.HandleInsertCommandSlice.InsertObjectCmObjectUi.CreateNewUiObject. (The one other HandleInsertCommand caller, InflAffixTemplateMenuHandler, is inside #if Later — compiled out.)
  • No configuration XML anywhere in DistFiles passes className="FsFeatDefn" to an insert command.
  • Even if XML did, Slice.InsertObject returns -1 for abstract classes before calling CreateNewUiObjectFsFeatDefn is abstract in the LCM model; MoForm is the only abstract class it special-cases (remapped to a concrete allomorph class).
  • MakeUi uses an explicit compiled switch (no reflection-by-name) and never constructs FsFeatDefnUi.
  • User-customized or extension-supplied XML cannot reach it either: the abstract-class gate lives in compiled code and the compiled LCM model, so a hand-added DataTreeInsert command with className="FsFeatDefn" was already a silent no-op before this change and behaves identically after it.

What stopped happening, and why each disappearance is safe

  • Nothing user-visible stopped. The deleted code could not execute on any path, shipped or customized.
  • Inserting inflection/phonological features still works through CmdInsertClosedFeature / CmdInsertComplexFeature / CmdInsertPhonologicalClosedFeature via RecordClerk.OnInsertItemInVector → the DialogInsertItemInVector Pub/Sub event → the Master*FeatDlgListeners, which are unaffected (their live BroadcastMessageUntilHandled("JumpToRecord", ...) sends remain).

Validation

  • .\build.ps1 -SkipNative: Build succeeded, 0 errors; FdoUi.dll and MorphologyEditorDll.dll rebuilt.
  • Re-grepped: no remaining references to FsFeatDefnUi or FsFeatDefnTags in Src/FdoUi.
  • No csproj edit needed (FdoUi is an SDK-style project with file globbing).

🤖 Generated with Claude Code


This change is Reviewable

FsFeatDefnUi was unreachable: its only external reference was the
FsFeatDefnTags.kClassId case in CmObjectUi.CreateNewUiObject, whose sole
call chain (DataTreeInsert command -> Slice.HandleInsertCommand ->
Slice.InsertObject) can never deliver that class id. No configuration
XML uses className="FsFeatDefn", and FsFeatDefn is abstract, so
Slice.InsertObject bails out before CreateNewUiObject is called (MoForm
is the only abstract class it special-cases). MakeUi never constructs
FsFeatDefnUi either. Inserting features works through the
CmdInsert*Feature commands via RecordClerk.OnInsertItemInVector and the
Master*FeatDlgListeners, which are unaffected.

Even user-customized or extension-supplied configuration XML could not
reach this code: the abstract-class check lives in compiled code and the
compiled LCM model, so a hand-added DataTreeInsert command with
className="FsFeatDefn" was already a silent no-op before this change,
and behaves identically after it.

Also removed commented-out JumpToRecord sends in
MasterPhonFeatDlgListener and MasterInflFeatDlgListener and folded the
comment lines that referred to them into a single accurate line.

Groundwork for the JumpToRecord Mediator-to-PubSub conversion: this
removes one of the seven synchronous SendMessage senders as dead code
before the conversion starts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.10%. Comparing base (f713635) to head (382f07a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1045   +/-   ##
=======================================
  Coverage   33.10%   33.10%           
=======================================
  Files        1201     1200    -1     
  Lines      277959   277937   -22     
  Branches    37130    37128    -2     
=======================================
+ Hits        92018    92019    +1     
+ Misses     158146   158126   -20     
+ Partials    27795    27792    -3     
Files with missing lines Coverage Δ
Src/FdoUi/FdoUiCore.cs 8.73% <ø> (+<0.01%) ⬆️
...rc/LexText/Morphology/MasterInflFeatDlgListener.cs 0.00% <ø> (ø)
...rc/LexText/Morphology/MasterPhonFeatDlgListener.cs 0.00% <ø> (ø)

... and 1 file 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.

@jasonleenaylor jasonleenaylor 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:

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

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