Skip to content

Share one implementation between the QPXML cdata/comment/pi getters - #104

Merged
jakejackson1 merged 1 commit into
mainfrom
fix/issue-96
Oct 25, 2026
Merged

jakejackson1 merged 1 commit into
mainfrom
fix/issue-96

Conversation

@gravitypdf-bot

Copy link
Copy Markdown
Contributor

Fixes #96

QPXML::cdata(), comment() and pi() each had their own copy of "the text of the first child node of type X in the match set". The copies had drifted: they mixed == and ===, and some returned null explicitly while others fell off the end. All three now call one private helper, firstChildText($nodeType, $target = null). pi() gains a full docblock, and "processor instruction" is corrected to "processing instruction".

There is no behaviour change, so there is no CHANGELOG entry.

Tests: I wrote these first and ran them green on the old code, then refactored:

  • testGettersReturnNullWhenNothingFound: no match, an empty set, and nested nodes that aren't direct children;
  • testGettersReturnFirstFoundAcrossMatches, including pi() with a target.

🤖 Generated with Claude Code

The three getters were copies of the same loop that had drifted: cdata()
used === and returned null explicitly, while comment() and pi() used ==
and fell off the end. Move the loop into a private firstChildText()
helper, keeping pi()'s filter on the processing instruction's target,
and give pi() a docblock to match the other two.

No behaviour change. Tests pin down the not-found, first-across-matches
and child-only cases for all three getters.

Fixes #96

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gravitypdf-bot gravitypdf-bot changed the title Share one implementation between the QPXML::cdata(), comment() and pi() getters Share one implementation between the QPXML cdata/comment/pi getters Oct 25, 2026
@codecov

codecov Bot commented Oct 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.35%. Comparing base (7ffe1aa) to head (cfa5d69).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #104      +/-   ##
============================================
+ Coverage     89.84%   90.35%   +0.51%     
+ Complexity     1430     1425       -5     
============================================
  Files            26       26              
  Lines          3189     3184       -5     
============================================
+ Hits           2865     2877      +12     
+ Misses          324      307      -17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jakejackson1
jakejackson1 merged commit 74c5b62 into main Oct 25, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-96 branch October 25, 2026 23:47
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.

Share one implementation between the QPXML::cdata(), comment() and pi() getters

2 participants