Sort renderer extensions by length to prioritize specific extensions #292
Conversation
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
Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
…nder Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] Address feedback on fix for template file searching
Sort renderer extensions by length to prioritize specific extensions
Jan 6, 2026
Member
|
@copilot PHP Fatal error: Class Yiisoft\View\PhpTemplateRenderer@anonymous cannot extend final class Yiisoft\View\PhpTemplateRenderer in /home/runner/work/view/view/tests/ViewTest.php on line 265 |
…lateRenderer Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
… new ones Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
Author
Fixed! Changed the test to implement |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 289-double-extensions #292 +/- ##
========================================================
Coverage 99.14% 99.14%
Complexity 311 311
========================================================
Files 19 19
Lines 815 816 +1
========================================================
+ Hits 808 809 +1
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes renderer selection when multiple renderers have overlapping file extensions. The key change ensures that more specific (longer) extensions like "blade.php" are matched before less specific (shorter) ones like "php", regardless of registration order.
Key Changes:
- Sorts renderer extensions by length in descending order when renderers are set
- Adds comprehensive test coverage for overlapping extension scenarios
- Optimizes performance by sorting once during registration rather than on each render
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ViewTrait.php | Added sorting logic in withRenderers() to prioritize longer extensions using uksort() with descending length comparison |
| tests/ViewTest.php | Added testOverlappingExtensionRendererPriority() test that verifies longer extensions are matched first by using anonymous classes implementing TemplateRendererInterface with identifying markers |
After a thorough review of the implementation, I found no issues with this pull request. The code is well-implemented with the following strengths:
✅ Correct sorting logic: Uses uksort() with strlen($b) <=> strlen($a) to ensure longer extensions are checked first
✅ Optimal performance: Sorting happens once during withRenderers() rather than on every render call
✅ Comprehensive test coverage: The new test properly validates the fix using anonymous classes with markers
✅ Clean implementation: Clear inline comment explaining the purpose, properly reuses PhpTemplateRenderer instance
✅ Edge case handling: Empty string extensions are already filtered in the render loop (line 455-456 in ViewTrait.php)
The implementation successfully addresses the issue described in the PR where overlapping extensions could cause unexpected behavior depending on registration order.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
This PR successfully addresses the feedback about renderer selection with overlapping extensions. All changes have been implemented and validated:
Predictable behavior: Longer, more specific extensions (like "blade.php") are always matched before shorter ones (like "php"), regardless of registration order.
Efficient implementation: Sorting happens once when renderers are set via
withRenderers(), not on every render call.Test coverage: Added
testOverlappingExtensionRendererPriority()to verify the correct behavior when multiple renderers with overlapping extensions are registered. The test properly implementsTemplateRendererInterfaceand efficiently reuses a singlePhpTemplateRendererinstance.Fixed fatal error: Changed test from extending the final
PhpTemplateRendererclass to implementingTemplateRendererInterfacedirectly.The implementation uses
uksort()to sort renderer extensions by length in descending order in thewithRenderers()method, ensuring both correctness and performance.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.