Skip to content

LT-22607 TonePars use XAmpleParser code for result processing#1002

Open
AndyBlack wants to merge 3 commits into
mainfrom
LT22607
Open

LT-22607 TonePars use XAmpleParser code for result processing#1002
AndyBlack wants to merge 3 commits into
mainfrom
LT22607

Conversation

@AndyBlack

@AndyBlack AndyBlack commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Now that the TonePars utility is part of FLEx, we can have it use the same code that XAmple does when processing its result.

  1. This change took the result processing of XAmpleParser and put it in a new public method.
  2. ToneParsInvoker now gets an instance of XAmpleParser (or creates it, if needed).
  3. The change removes the duplicated code from ToneParsInvoker.
  4. ToneParsFLExForm passes in an instance of XAmpleParser.

This change is Reviewable

AndyBlack added 2 commits July 9, 2026 12:24
Change-Id: Id4eabbcd70df8b594d26ee2fba777783f518f070
Change-Id: I89db58bf48ffb14b8c8edcaed8ae26549a7a522a
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   11m 0s ⏱️ +37s
4 302 tests ±0  4 229 ✅ ±0  73 💤 ±0  0 ❌ ±0 
4 311 runs  ±0  4 238 ✅ ±0  73 💤 ±0  0 ❌ ±0 

Results for commit 0d94b9f. ± Comparison against base commit 6a73c08.

♻️ This comment has been updated with latest results.

@johnml1135 johnml1135 self-requested a review July 9, 2026 22:09

@johnml1135 johnml1135 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.

Claude - Sonnet 5 — automated code review.

Two additional findings sit on lines outside this PR's diff hunks (pre-existing code untouched here), so they can't be attached as inline comments — noting them here instead:

  1. Reuse (Src/Utilities/pcpatrflex/ToneParsFLExDll/ToneParsInvoker.cs ~line 101): ConvertNameToUseAnsiCharacters is a byte-for-byte duplicate of XAmpleParser.ConvertNameToUseAnsiCharacters, explicitly commented as "borrowed from SIL.FieldWorks.WordWorks.Parser (ParserCore.dll)". Since this PR already dedupes ProcessParseResults against the same class, and ToneParsInvoker now holds a live XAmpleParser reference, this is a natural candidate to dedupe too (note: XAmpleParser's version is internal static, so calling through would need an accessibility change).
  2. Cleanup (Src/Utilities/pcpatrflex/ToneParsFLExDll/ToneParsInvoker.cs line 22): using System.Xml.Linq; is now dead — it was the only consumer's (XElement/Descendants) namespace, and that code was deleted by this PR. No remaining use of any System.Xml.Linq type in the file.

Comment thread Src/Utilities/pcpatrflex/ToneParsFLExDll/ToneParsInvoker.cs Outdated
Comment thread Src/LexText/ParserCore/XAmpleParser.cs Outdated
Change-Id: Iec04860ae004f83adf3c64cf9d54848ecf423906
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.

2 participants