fix: take phiki's own offset fix and drop the local patch - #61
Merged
Merged
Conversation
CI has been red on every branch since phiki 2.2.1: the post-update hook ran scripts/patch-phiki-offsets.php, the anchor it rewrites no longer existed, and the script exited 1 - which fails composer install itself, so every job died before running anything. composer.lock is not tracked here, so CI always resolved the new phiki and always hit it. The anchor moved because the fix is upstream now. phiki 2.2.1's PatternSearcher carries the same cursor walk the patch applied, down to the sibling check and the fallback, under its own names (previousStart / previousEnd). Patching it again is neither possible nor wanted. Evidence this is safe: the upstream source implements the same algorithm, and the suite passes on 2.2.1 with no patch. I could not build an input that tells patched 2.2.0 from unpatched 2.2.0, so the A/B I ran proves nothing either way and is not offered as support.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the local phiki offset patching mechanism now that the fix has landed upstream, unblocking composer install and CI by eliminating a failing Composer post-update hook.
Changes:
- Deletes the
scripts/patch-phiki-offsets.phppatch script. - Removes
post-install-cmd/post-update-cmdComposer hooks that executed the patch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/patch-phiki-offsets.php | Removes the now-unneeded local vendor patch script. |
| composer.json | Drops Composer lifecycle hooks that ran the patch script. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
44
to
49
| "scripts": { | ||
| "cs-check": "phpcs --colors --parallel=16", | ||
| "cs-fix": "phpcbf --colors --parallel=16", | ||
| "stan": "phpstan analyze", | ||
| "test": "phpunit", | ||
| "post-install-cmd": [ | ||
| "php scripts/patch-phiki-offsets.php" | ||
| ], | ||
| "post-update-cmd": [ | ||
| "php scripts/patch-phiki-offsets.php" | ||
| ] | ||
| "test": "phpunit" | ||
| } |
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.
CI is red on every branch, including #58 and #59, and has been since phiki 2.2.1 was released.
The script runs from composer's post-update hook, so its exit 1 fails composer install itself and every job dies before running a single test.
composer.lockis not tracked here, so CI always resolves the new phiki and always hits it.Why the anchor moved
Because the fix is upstream. phiki 2.2.1's
PatternSearchercarries the same cursor walk the patch applied - same sibling check, same fallback - under its own names (previousStart/previousEnd). Patching it a second time is neither possible nor wanted.So the script and its two composer hooks go.
Evidence this is safe
composer test(107),stanandcs-checkall pass on 2.2.1 with no patch, from a cleancomposer install.I also tried to A/B patched 2.2.0 against unpatched 2.2.0 and could not build an input that distinguishes them - so that comparison proves nothing either way, and I am not offering it as support. If you know the input that originally showed the wrong offset, that would be the thing to check.
Merging this unblocks #58 and #59.