Skip to content

Make detach($selector) honour its selector - #108

Merged
jakejackson1 merged 1 commit into
mainfrom
fix/issue-76
Oct 26, 2026
Merged

jakejackson1 merged 1 commit into
mainfrom
fix/issue-76

Conversation

@gravitypdf-bot

Copy link
Copy Markdown
Contributor

detach() called find($selector) and threw the result away, so it always detached the current match set:

$qp = qp('<?xml version="1.0"?><root><p><i>a</i></p></root>', 'p');
$qp->detach('i')->tag();   // before: 'p', and <root/> is left
                           // after:  'i', and <root><p/></root> is left

Changes

  • detach($selector) now detaches the selector's matches. The current match set stays in the document and $qp still wraps it, as with remove($selector).
  • The detached nodes are recorded as $qp's previous match set, so attach() re-attaches the nodes that were actually detached. With no selector, nothing changes.

Tests

New tests in DOMQueryTest check:

  • the returned object holds only the detached i elements;
  • the p elements and their other children stay in the document;
  • end() on the returned object goes back to the p set;
  • attach() re-attaches the detached nodes;
  • detach() and remove() leave the same document and return the same count for i, p > i, p, * and an unmatched selector.

Without the fix, three of these tests fail. The full suite, lint and lint:min-php pass.

detach() and remove() still differ on end(). end() on the object detach() returns gives back the original match set, because detach() builds its result with inst(). remove() returns a fresh object, so end() on its result is empty. This PR leaves that unchanged.

Fixes #76.

🤖 Generated with Claude Code

detach() discarded the result of find(), so it always detached the
current match set. Detach the selector's matches instead, leaving the
current match set in the document, as remove($selector) does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.72%. Comparing base (6ae6acf) to head (77bd3bc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #108      +/-   ##
============================================
+ Coverage     90.69%   90.72%   +0.02%     
  Complexity     1424     1424              
============================================
  Files            25       25              
  Lines          3170     3169       -1     
============================================
  Hits           2875     2875              
+ Misses          295      294       -1     

☔ 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 added the bug Something isn't working label Oct 26, 2026
@jakejackson1
jakejackson1 merged commit 0eb7c1d into main Oct 26, 2026
14 checks passed
@jakejackson1
jakejackson1 deleted the fix/issue-76 branch October 26, 2026 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

detach($selector) ignores its selector and detaches the current match set instead

2 participants