From 354f7fcd3575850edbfdf68c6df6ddc86af9577c Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 16:59:36 +1100 Subject: [PATCH 1/4] Count negative indexes from the end in eq(), get() and slice() 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 --- src/DOM.php | 14 +++++++--- src/DOMQuery.php | 5 ++-- src/Helpers/QueryFilters.php | 11 +++++--- tests/Issues/Issue142Test.php | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 tests/Issues/Issue142Test.php diff --git a/src/DOM.php b/src/DOM.php index dbd889b..6af548b 100644 --- a/src/DOM.php +++ b/src/DOM.php @@ -621,14 +621,20 @@ protected function isXMLish($string) * The internal data structure used in DOMQuery does not have * strong random access support, so we suppliment it with this method. * - * @param $index + * @param int $index + * Zero-based. A negative index counts back from the end of the matches. * - * @return object|void + * @return object|null + * NULL if the index is out of range. */ protected function getNthMatch(int $index) { - if ($index < 0 || $index > $this->matches->count()) { - return; + $count = $this->matches->count(); + if ($index < 0) { + $index += $count; + } + if ($index < 0 || $index >= $count) { + return null; } $i = 0; diff --git a/src/DOMQuery.php b/src/DOMQuery.php index 4c9028b..b4cddd2 100644 --- a/src/DOMQuery.php +++ b/src/DOMQuery.php @@ -266,7 +266,8 @@ public function count(): int * in QueryPath, see {@link examples/techniques.php}. * * @param int|null $index - * If specified, then only this index value will be returned. If this + * If specified, then only this index value will be returned. A negative + * index counts back from the end, so -1 is the last match. If this * index is out of bounds, a NULL will be returned. * @param boolean $asObject * If this is TRUE, an SplObjectStorage object will be returned @@ -281,7 +282,7 @@ public function count(): int public function get($index = null, $asObject = false) { if ($index !== null) { - return ($this->count() > $index) ? $this->getNthMatch($index) : null; + return $this->getNthMatch($index); } // Retain support for legacy. if (! $asObject) { diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 8b73c7b..419f08f 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -263,7 +263,8 @@ public function map($callback): Query * Narrow the items in this object down to only a slice of the starting items. * * @param integer $start - * Where in the list of matches to begin the slice. + * Where in the list of matches to begin the slice. A negative index counts + * back from the end. * @param integer $length * The number of items to include in the slice. If nothing is specified, the * all remaining matches (from $start onward) will be included in the sliced @@ -280,6 +281,9 @@ public function slice($start, $length = 0): Query if ($start >= $this->count()) { return $this->inst($found, null); } + if ($start < 0) { + $start += $this->count(); + } $i = $j = 0; foreach ($this->matches as $m) { @@ -620,8 +624,9 @@ public function parentsUntil($selector = null): Query * with {@link end()}. * * @param $index - * The index of the element to keep. The rest will be - * discarded. + * The zero-based index of the item to keep. The rest will be + * discarded. A negative index counts back from the end, so -1 keeps + * the last item. An index out of range gives an empty set. * * @return DOMQuery * @throws ParseException diff --git a/tests/Issues/Issue142Test.php b/tests/Issues/Issue142Test.php new file mode 100644 index 0000000..89f07bd --- /dev/null +++ b/tests/Issues/Issue142Test.php @@ -0,0 +1,48 @@ +'; + + public function testEqCountsANegativeIndexFromTheEnd() + { + $qp = qp(self::XML, 'i'); + + $this->assertSame('e', $this->idList($qp->eq(-1))); + $this->assertSame('d', $this->idList($qp->eq(-2))); + $this->assertSame('a', $this->idList($qp->eq(-5))); + $this->assertSame('', $this->idList($qp->eq(-6))); + $this->assertSame('', $this->idList($qp->eq(5))); + $this->assertSame('', $this->idList(qp(self::XML, 'nothing')->eq(-1))); + } + + public function testGetCountsANegativeIndexFromTheEnd() + { + $qp = qp(self::XML, 'i'); + + $this->assertSame('e', $qp->get(-1)->getAttribute('id')); + $this->assertSame('a', $qp->get(-5)->getAttribute('id')); + $this->assertNull($qp->get(-6)); + $this->assertNull($qp->get(5)); + $this->assertNull(qp(self::XML, 'nothing')->get(-1)); + } + + public function testSliceCountsANegativeStartFromTheEnd() + { + $qp = qp(self::XML, 'i'); + + $this->assertSame('d,e', $this->idList($qp->slice(-2))); + $this->assertSame('e', $this->idList($qp->slice(-1))); + $this->assertSame('a,b,c,d,e', $this->idList($qp->slice(-5))); + $this->assertSame('a,b,c,d,e', $this->idList($qp->slice(-9))); + } +} From ace1f948b3edb929d5f14ecbffd249be64b2983c Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:00:15 +1100 Subject: [PATCH 2/4] Take an exclusive end index as slice()'s second argument, as jQuery does 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 --- src/Helpers/QueryFilters.php | 42 +++++++++++++++---------------- tests/Issues/Issue144Test.php | 43 ++++++++++++++++++++++++++++++++ tests/QueryPath/DOMQueryTest.php | 2 +- 3 files changed, 65 insertions(+), 22 deletions(-) create mode 100644 tests/Issues/Issue144Test.php diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 419f08f..48011d4 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -262,37 +262,37 @@ public function map($callback): Query /** * Narrow the items in this object down to only a slice of the starting items. * - * @param integer $start - * Where in the list of matches to begin the slice. A negative index counts - * back from the end. - * @param integer $length - * The number of items to include in the slice. If nothing is specified, the - * all remaining matches (from $start onward) will be included in the sliced - * list. + * As in jQuery's .slice(), the slice runs from $start up to, but not + * including, $end. Both are zero-based indexes, and a negative index counts + * back from the end, so slice(1, -1) drops the first and last items. + * + * @param integer $start + * The index of the first item to include. + * @param integer|null $end + * The index to stop before. If omitted, the slice runs to the end of the + * matches. An $end at or before $start gives an empty set. * * @return DOMQuery * @throws ParseException - * @see array_slice() */ - public function slice($start, $length = 0): Query + public function slice($start, $end = null): Query { - $end = $length; - $found = new SplObjectStorage(); - if ($start >= $this->count()) { - return $this->inst($found, null); - } - if ($start < 0) { - $start += $this->count(); + $count = $this->count(); + $start = $start < 0 ? $start + $count : $start; + if ($end === null) { + $end = $count; + } elseif ($end < 0) { + $end += $count; } - $i = $j = 0; + $found = new SplObjectStorage(); + $i = 0; foreach ($this->matches as $m) { + if ($i >= $end) { + break; + } if ($i >= $start) { - if ($end > 0 && $j >= $end) { - break; - } $found->offsetSet($m); - ++$j; } ++$i; } diff --git a/tests/Issues/Issue144Test.php b/tests/Issues/Issue144Test.php new file mode 100644 index 0000000..33ecd9b --- /dev/null +++ b/tests/Issues/Issue144Test.php @@ -0,0 +1,43 @@ +'; + + /** + * @dataProvider sliceProvider + */ + public function testSliceTakesAnEndIndex(array $args, string $expected) + { + $this->assertSame($expected, $this->idList(qp(self::XML, 'i')->slice(...$args))); + } + + public function sliceProvider(): array + { + return [ + 'slice(1, 3)' => [[1, 3], 'b,c'], + 'slice(1, -1)' => [[1, -1], 'b,c,d'], + 'slice(2, 0)' => [[2, 0], ''], + 'slice(0)' => [[0], 'a,b,c,d,e'], + 'slice(2)' => [[2], 'c,d,e'], + 'slice(1, 9)' => [[1, 9], 'b,c,d,e'], + 'slice(3, 2)' => [[3, 2], ''], + 'slice(2, 2)' => [[2, 2], ''], + 'slice(-3, -1)' => [[-3, -1], 'c,d'], + 'slice(-2, 5)' => [[-2, 5], 'd,e'], + 'slice(0, -5)' => [[0, -5], ''], + 'slice(0, -9)' => [[0, -9], ''], + 'slice(-9, 2)' => [[-9, 2], 'a,b'], + 'slice(5)' => [[5], ''], + ]; + } +} diff --git a/tests/QueryPath/DOMQueryTest.php b/tests/QueryPath/DOMQueryTest.php index 674f925..90e10b6 100644 --- a/tests/QueryPath/DOMQueryTest.php +++ b/tests/QueryPath/DOMQueryTest.php @@ -1011,7 +1011,7 @@ public function testSlice() $this->assertEquals(0, qp($file, 'li')->slice(9)->count()); // The first item should be #two, the last #three - $qp = qp($file, 'li')->slice(1, 2); + $qp = qp($file, 'li')->slice(1, 3); $this->assertEquals(2, $qp->count()); $this->assertEquals('two', $qp->attr('id')); $this->assertEquals('three', $qp->eq(1)->attr('id')); From 7fa21457f4985dbe3e2cf5d12a5896631acc99da Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:17:21 +1100 Subject: [PATCH 3/4] Add a CHANGELOG entry Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527a87d..99427d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ QueryPath Changelog - Consolidate the sibling and ancestor walks behind `next()`, `prev()`, `nextAll()`, `prevAll()`, `nextUntil()`, `prevUntil()`, `parent()`, `parents()`, `parentsUntil()` and `closest()` into one helper, so a fix to one no longer has to be repeated in its mirror (issue #115). As a side effect, `parentsUntil()` no longer loops forever on an element that is not attached to the document, and `nextUntil('')` and `prevUntil('')` treat an empty selector as no selector, as the other methods already did, instead of throwing `QueryPath\CSS\ParseException`. `closest('')` returns an empty set, as in jQuery. `nextUntil()`, `prevUntil()` and `parentsUntil()` also stop at an element already collected from an earlier starting element, as `nextAll()` and `prevAll()` do, so `nextUntil()` on each of 2000 siblings falls from about 100 ms to 1 ms - **Behaviour change.** `prevAll()`, `prevUntil()`, `nextAll()`, `nextUntil()`, `siblings()`, `parent()` and `closest()` now return the results for a multi-element set in jQuery's order, instead of grouped by starting element (issue #117). `prevAll()` and `prevUntil()` return reverse document order, as `parents()` and `parentsUntil()` already did, and the others return document order. `qp($xml, 'c')->add('d')->prevAll()` now returns `c, b, a` rather than `b, a, c`. `next()` and `prev()` keep the order of the starting elements, as in jQuery, and the results for a single starting element are unchanged - **Behaviour change.** A positional pseudo-class (`:first`, `:eq()`, `:last`, ...) in the stop selector of `nextUntil()`, `prevUntil()` and `parentsUntil()` now counts across the whole document, as jQuery's does, instead of seeing each sibling or ancestor as a one-element set (issue #118). `nextUntil(':first')` used to stop at once and now walks to the end, and `nextUntil(':eq(3)')` stops at the fourth element in the document. `closest()` no longer matches a positional selector at all, as in jQuery, where `closest(':first')` used to return the element itself. `next()`, `prev()` and `parent()` still test each candidate on its own, which agrees with jQuery for a single starting element +- **Behaviour change.** `eq()`, `get()` and `slice()` now count a negative index back from the end of the match set, as jQuery does (issue #142). `eq(-1)` and `get(-1)` are the last match, where they used to give an empty set and `NULL`. `slice(-2)` now returns the last two matches instead of the whole set. An index that is still out of range gives an empty set, or `NULL` from `get()` +- **Breaking behaviour change.** `slice($start, $end)` now takes an exclusive end index as its second argument, as jQuery's `.slice()` does, instead of a length (issue #144). `slice(1, 3)` returns the second and third matches, where it used to return three matches. A negative `$end` counts back from the end, so `slice(1, -1)` drops the first and last matches. Leaving out `$end` still slices to the end of the set, but an explicit `0`, which used to mean "to the end", now gives an empty set. To keep a length, use `slice($start, $start + $length)` # 4.2.0 From 47a9fe71cb4d0a0f9614574b1903ac01f2910f05 Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Mon, 26 Oct 2026 17:25:05 +1100 Subject: [PATCH 4/4] Resolve negative indexes through Util, as :eq() does Co-Authored-By: Claude Opus 5.5 --- src/CSS/DOMTraverser/Util.php | 4 ++-- src/DOM.php | 5 ++--- src/Helpers/QueryFilters.php | 17 ++++------------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/CSS/DOMTraverser/Util.php b/src/CSS/DOMTraverser/Util.php index 033b14a..62746ee 100644 --- a/src/CSS/DOMTraverser/Util.php +++ b/src/CSS/DOMTraverser/Util.php @@ -246,14 +246,14 @@ public static function sortDocumentOrder(array $nodes): array } /** - * Resolve a (possibly negative) positional index against a set size. + * Resolve a zero-based index against a set size, counting a negative one back from the end. * * @param mixed $value * @param int $count * * @return int */ - private static function normalizePositionalIndex($value, int $count): int + public static function normalizePositionalIndex($value, int $count): int { $index = (int) $value; diff --git a/src/DOM.php b/src/DOM.php index 6af548b..b129141 100644 --- a/src/DOM.php +++ b/src/DOM.php @@ -10,6 +10,7 @@ use IteratorAggregate; use Masterminds\HTML5; use QueryPath\CSS\DOMTraverser; +use QueryPath\CSS\DOMTraverser\Util; use QueryPath\Entities; use SimpleXMLElement; use SplObjectStorage; @@ -630,9 +631,7 @@ protected function isXMLish($string) protected function getNthMatch(int $index) { $count = $this->matches->count(); - if ($index < 0) { - $index += $count; - } + $index = Util::normalizePositionalIndex($index, $count); if ($index < 0 || $index >= $count) { return null; } diff --git a/src/Helpers/QueryFilters.php b/src/Helpers/QueryFilters.php index 48011d4..910222e 100644 --- a/src/Helpers/QueryFilters.php +++ b/src/Helpers/QueryFilters.php @@ -278,23 +278,14 @@ public function map($callback): Query public function slice($start, $end = null): Query { $count = $this->count(); - $start = $start < 0 ? $start + $count : $start; - if ($end === null) { - $end = $count; - } elseif ($end < 0) { - $end += $count; - } + $start = max(0, Util::normalizePositionalIndex($start, $count)); + $end = $end === null ? $count : Util::normalizePositionalIndex($end, $count); $found = new SplObjectStorage(); - $i = 0; - foreach ($this->matches as $m) { - if ($i >= $end) { - break; - } - if ($i >= $start) { + if ($start < $end) { + foreach (array_slice(iterator_to_array($this->matches, false), $start, $end - $start) as $m) { $found->offsetSet($m); } - ++$i; } return $this->inst($found, null);