Skip to content

refactor: build clip XML via XElement instead of string interpolation#222

Merged
fuzzzerd merged 1 commit into
masterfrom
fuzzz/xelement-not-interpolation
May 20, 2026
Merged

refactor: build clip XML via XElement instead of string interpolation#222
fuzzzerd merged 1 commit into
masterfrom
fuzzz/xelement-not-interpolation

Conversation

@fuzzzerd
Copy link
Copy Markdown
Owner

Summary

Replaces nine string-interpolated XML construction sites with XElement / XAttribute / XCData API calls. The framework handles attribute escaping and CDATA wrapping; manual escaping is no longer needed.

Why

TableClipStrategy.DefaultXml previously interpolated a clip name into an XML attribute via $"<BaseTable name=\"{clipName}\">". A clip named My "favorite" stuff produced malformed XML; a recent fix patched the symptom by calling a hand-rolled XmlHelpers.XmlEscape before interpolation. Auditing for the same anti-pattern surfaced eight more sites: Calculation.ToXml and seven XElement.Parse($"<X><![CDATA[{userText}]]></X>") callsites in FmField.ToXml. All share the same root cause — building structured XML by string concatenation — and the same class of edge-case failure (e.g. ]]> inside calc text would terminate the CDATA early and throw on parse).

Scope

  • TableClipStrategy.DefaultXmlnew XElement("BaseTable", new XAttribute("name", clipName))
  • Calculation.ToXmlnew XElement(name, new XCData(Text))
  • FmField.ToXml (×7: Calculation, ConstantData, MinimumValue, MaximumValue, StrictValidation, ErrorMessage, AutoEnter-Calculation) → same XCData pattern
  • XmlHelpers.XmlEscape deleted (no callers remain)
  • IClipTypeStrategy.DefaultXml doc-comment updated to point at XElement rather than at a manual-escape helper

new XCData(text) still can't legally hold the literal sequence ]]> — that's an XML spec limitation, not an API one. The failure mode improves from "silent on-disk corruption" to "throw on save," which is acceptable; FileMaker's own calc grammar can't produce ]]> either.

Tests

All 1424 existing tests still pass. Added 3 new Calculation.RoundTrip_PreservesXmlMetacharacters cases covering <, &, ", ' in calc text. Added one O'Brien Theory row to Table_DefaultXml_EscapesPunctuationInName.

Follow-up

The display-text path uses a different escape contract (FileMaker's own quoting convention rather than XML escaping) and is tracked separately in #221 — script/layout/menu-set names containing " produce ambiguous display text. Round-trip probably still works via greedy regex but isn't covered by tests; lower priority than this fix.

@fuzzzerd fuzzzerd enabled auto-merge (rebase) May 20, 2026 03:29
@github-actions
Copy link
Copy Markdown

Test Results

✔️ Tests 1427 / 1427 - passed in 19.2s
✔️ Coverage 79.42% - passed with 70% threshold
📏 15266 / 17688 lines covered 🌿 5192 / 8072 branches covered
🔍 click here for more details

✏️ updated for commit d9fcaf7

@fuzzzerd fuzzzerd merged commit a3fce1d into master May 20, 2026
6 checks passed
@fuzzzerd fuzzzerd deleted the fuzzz/xelement-not-interpolation branch May 20, 2026 03:31
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.

1 participant