Skip to content

Fix SetText replacement after leading comments - #1095

Open
fhgffy wants to merge 1 commit into
leethomason:masterfrom
fhgffy:fix/settext-leading-comments
Open

fhgffy wants to merge 1 commit into
leethomason:masterfrom
fhgffy:fix/settext-leading-comments

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 3, 2026 •

Copy link
Copy Markdown

When an element starts with comments, SetText() currently inserts a new text node before the comments and leaves the previous value in the tree. For <foo><!--comment-->old</foo>, setting "new" serializes as <foo>new<!--comment-->old</foo>.

Skip leading comments when selecting the text node, matching GetText(), and replace that existing node. Keep first-child insertion when no text node follows the comments. Add regression coverage for consecutive comments, mixed content, CDATA, numeric setters, and the insertion fallback, and update the SetText() documentation.

Fixes #927.

Validation on Windows with GCC 15.1.0, CMake 4.3.2, and Ninja 1.13.2:

  • The minimal reproducer fails on master 8224e427b655b83dae5e2298f1e6919523a78737 (exit 1) and passes with the fix (exit 0).
  • The original suite passes 528 checks. With only the regression tests added, 12 checks fail; after the fix, all 558 checks pass in Debug and Release.
  • CMake/Ninja static/shared Debug/Release builds and CTest all pass. The local MinGW Makefiles generator failed to handle the workspace's Unicode absolute path; Ninja was used for the CMake checks.

@fhgffy
fhgffy marked this pull request as ready for review October 3, 2026 23:41

This branch has not been deployed

No deployments
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.

XMLElement::SetText needs to skip any leading comments like XMLElement::GetText

1 participant