Repository navigation
Fix open issues #132–#146 - #147
Closed
gravitypdf-bot wants to merge 19 commits into
Closed
gravitypdf-bot wants to merge 19 commits into
gravitypdf-bot wants to merge 19 commits into
Conversation
siblings() read the parent's childNodes without checking for a parent, so an element outside any tree raised two warnings. Fixes #139. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
replaceAll() made a shallow clone of the replacement and a shallow import of that, so each replaced node arrived with its attributes but no children. Import the replacement deeply instead. Fixes #135 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getNthMatch() returned nothing for a negative index, so eq(-1) was empty and get(-1) NULL, and slice() kept every match from a negative start. As in jQuery, a negative index now counts back from the end of the set. Fixes #142. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
slice($start, $length) followed array_slice(), with a length of 0 meaning "to the end". It is now slice($start, $end), as in jQuery: the slice stops before $end, a negative $end counts back from the end, and an omitted $end runs to the end of the set. Fixes #144. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nk() Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in filter()
In filterNodes(), a non-subject compound carrying a positional
pseudo-class was resolved by searching the candidates' descendants, so
filter('ul:first > li') and is('ul:first > li') matched nothing. It is
now resolved from the document element, as jQuery's .filter() does.
Fixes #132.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
even() kept positions 1, 3, 5 and odd() kept 0, 2, 4, the reverse of jQuery's .even() and .odd() and of QueryPath's own :even and :odd pseudo-classes. Both now go through one helper. Fixes #145. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
inst() cloned the object, and __clone() deep-copied every match with cloneNode(true), only for inst() to swap the match set straight after. Each find() from the root copied the whole document, and every chained call copied whatever was in hand. The copies also became the previous match set, so end() returned detached copies rather than the nodes the query started from. inst() now marks the clone as shallow so __clone() skips the copy; clone $qp still deep-copies the matches. Fixes #137 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jQuery's .is() evaluates a positional selector against the document and
tests membership, so is(':first') asks whether the set holds the
document's first element. is() counted within the match set instead,
as filter() does. It now uses NodeMatcher::matchesInDocument().
Fixes #140.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
even() and odd() counted text, comment, CDATA and processing-instruction nodes but dropped them from the result, so contents()->odd() could be empty. first() and last() skipped them altogether, so first() disagreed with eq(0). As in jQuery, all four now treat every node in the set alike, and first() and last() are eq(0) and eq(-1). Fixes #143. Fixes #146. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alk order
jQuery joins the walk from each starting element, repeats included,
filters that list with jQuery.filter(), then removes the repeats and
sorts. QueryPath counted a positional selector in document order, so
prevAll(':first') was the first sibling rather than the nearest.
Add DOMTraverser::filterList(), which counts positional pseudo-classes
along a list in the order given, and filter the joined walks through it
when the selector is positional.
Fixes #134.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e pass for plain selectors Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #147 +/- ##
============================================
+ Coverage 90.69% 91.66% +0.96%
- Complexity 1424 1469 +45
============================================
Files 25 26 +1
Lines 3170 3238 +68
============================================
+ Hits 2875 2968 +93
+ Misses 295 270 -25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes every open issue: #132, #133, #134, #135, #136, #137, #138, #139, #140, #142, #143, #144, #145, #146.
Where an issue offered a choice between matching jQuery and documenting the current behaviour (#140, #143, #144, #145, #146), this PR matches jQuery, consistent with the behaviour changes already listed under
# Unreleased changes. Each one has a Behaviour change CHANGELOG entry.Index and position methods
eq(),get()andslice()ignore negative indexes;slice(-2)returns the whole set #142eq(),get()andslice()count a negative index back from the end (DOM::getNthMatch()).slice()takes a length as its second argument, where jQuery takes an end index #144slice($start, $end)takes an exclusive end index, as jQuery does. This is a breaking change;slice($start, $start + $length)keeps the old result.even()andodd()are swapped relative to jQuery and the:even/:oddpseudo-classes #145even()/odd()count from zero, as jQuery's methods and:even/:odddo. This is a breaking change.even()andodd()count text nodes but drop them, socontents()->odd()can be empty #143,first()andlast()skip text and other non-element nodes, unlike jQuery andeq(0)#146even(),odd(),first()andlast()count and keep every node.first()/last()are noweq(0)/eq(-1).siblings()skips a match that has no parent, so it no longer raises warnings.Positional selectors
ul:first > li) #132 Infilter()/is(), a positional compound left of a combinator (ul:first > li) now counts across the document, as it does infind().is()counts positional selectors across the document, as jQuery's.is()does, via the newNodeMatcher::matchesAnyInDocument(). Selectors without a positional pseudo-class still take a single set pass.prevAll(),parents()andnextAll(), a positional selector counts in walk order (prevAll(':first')is the nearest sibling). This addsDOMTraverser::filterList(), which counts along a list in the order given.CSS engine
:scopealways refers to the document element, sofind(':scope > li')matches nothing #133 Infind(),findInPlace(),remove($selector)and theqp($node, $selector)constructor,:scopeis now the element the search starts from, including inside:not()/:matches(). When the set has several elements and the selector uses:scope, the search runs from each element, as jQuery's does.PseudoClass::is()/isNot()take a new optional$scopeargument.:is()and:where()as aliases of:matches()#136:is()and:where()are aliases of:matches().SimpleSelector::notEmpty()is deprecated; it now returns! isBlank().Cloning
replaceAll()copies each replacement with its children.inst()no longer deep-copies the match set it is about to replace. On a 50,000-paragraph document,children()from the root drops from about 27 ms to about 12 ms. A side effect:end()now returns the original nodes instead of detached copies.clone $qpstill deep-copies.Tests
tests/Issues/(Issue132Test.php…Issue146Test.php), covering the issue tables. CI runs them on PHP 7.1–8.5.DOMQueryTest::testSlice,testEven/testOdd,testFirstAndLastWithNonElementNodes, andIssue118Test's positional-order test, which now uses jQuery's walk-order values.Notes
siblings(':first')on a multi-element set still counts in document order.filterNodes()counts in document order rather than in the order its input is given.composer run lintandlint:min-phppass.🤖 Generated with Claude Code