Skip to content

Segment Create/SplitSegment/MergeSegments/RebuildSegments/Duplicate -- replace manual SegmentsOS mutation with IStTxtPara.Contents edits #174

@MattGyverLee

Description

@MattGyverLee

Root cause

ISegment.BaselineText is a read-only, computed ITsString property (declared in InterfaceAdditions.cs:542; implemented in OverridesCellar.cs:4397-4400). Its backing store is IStTxtPara.Contents.

The following methods in flexlibs2/code/TextsWords/SegmentOperations.py all follow the same broken pattern:

  1. Manually add/remove/restructure entries in para_obj.SegmentsOS (or call SegmentsOS.Clear()).
  2. Then try to set segment.BaselineText.set_String(...) or segment.BaselineText.CopyAlternatives(...) on the new/moved segments.

Both steps are wrong. SegmentsOS is owned by AnalysisAdjuster, not caller code. The correct LCM idiom is to edit IStTxtPara.Contents via a TsStrBldr; the ContentsSideEffects notification fires AnalysisAdjuster.AdjustAnalysis which rebuilds and reconciles SegmentsOS automatically.

Entangled sites (file:line)

All in flexlibs2/code/TextsWords/SegmentOperations.py:

Method Line(s) Nature of entanglement
Create 519 (SegmentsOS.Add) + 523 (set_String) Creates segment, manually adds to SegmentsOS, then tries to set BaselineText
Duplicate 634 (SegmentsOS.Add) + 638 (CopyAlternatives) Adds to SegmentsOS, then calls CopyAlternatives on ITsString (IMultiString method)
SplitSegment 890, 897 (SegmentsOS.Insert) + 892, 899 (set_String) Inserts two new segments, sets text on each
MergeSegments 992 (SegmentsOS.Insert) + 995 (set_String) Creates merged segment, inserts into SegmentsOS, sets text
RebuildSegments 1340 (SegmentsOS.Clear) + 1363 (SegmentsOS.Add) + 1367 (set_String) Clears all segments, creates new ones, sets text on each

Correct write idiom (from /lex-domain analysis)

para = segment_obj.Paragraph        # IStTxtPara
begin = segment_obj.BeginOffset
end = segment_obj.EndOffset
bldr = para.Contents.GetBldr()
new_run = TsStringUtils.MakeString(new_text, ws)
bldr.ReplaceTsString(begin, end, new_run)
para.Contents = bldr.GetString()    # ContentsSideEffects -> AnalysisAdjuster.AdjustAnalysis

For Create/Split/Merge/Rebuild the equivalent idiom is to build the full desired Contents string for the paragraph and assign it once; AnalysisAdjuster handles all segment reconciliation.

Relevant liblcm sources

  • InterfaceAdditions.cs:542 -- ISegment.BaselineText declaration (ITsString, get-only)
  • OverridesCellar.cs:4397-4400 -- computed implementation returning substring of para Contents
  • StTxtPara.cs:533-577 -- ContentsSideEffects triggering AnalysisAdjuster.AdjustAnalysis

(Source tree: D:\Github\_Projects\_LEX\liblcm\)

Dependency: test coverage

QC review of commit 295f613 noted there are currently no live tests for Duplicate, SplitSegment, or MergeSegments. These methods must have live-test coverage established before the architectural rewrite can be safely validated. Recommend filing or reusing a test-coverage issue alongside this work.

Context

Spun out of #172 after /lex-domain analysis identified that applying the bldr.ReplaceTsString fix mechanically to these methods would fight AnalysisAdjuster and likely regress. The safe read/write sites from #172 (GetSyncableProperties and SetBaselineText) were fixed directly in #172's commit; the entangled sites are deferred here.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions