-
-
Notifications
You must be signed in to change notification settings - Fork 49
Enh #289: Add validation of renderers configuration to ViewTrait::withRenderers()
#291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
============================================
+ Coverage 99.50% 99.51% +0.01%
- Complexity 305 310 +5
============================================
Files 19 19
Lines 800 822 +22
============================================
+ Hits 796 818 +22
Misses 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #289 by enabling the view system to properly detect and use template renderers with double file extensions such as "blade.php". Previously, the system only recognized single-part extensions, causing renderers registered with double extensions to not be matched correctly.
- Refactored test data provider to use named array keys for better readability
- Updated renderer selection logic to match extensions using suffix checking instead of pathinfo
- Enhanced template file searching to check registered renderer extensions before fallback extensions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/ViewTest.php | Refactored test data provider to use named keys, removed unused parameter, and added new test case for double extension rendering |
| src/ViewTrait.php | Modified renderer selection to use str_ends_with() for extension matching and added renderer extension checking in template file search |
| CHANGELOG.md | Documented the bug fix for double extension support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…292) Co-authored-by: samdark <47294+samdark@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: StyleCI Bot <bot@styleci.io>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
ViewTrait::withRenderers()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function testWithRenderersInvalidRendererTypeThrowsException(): void | ||
| { | ||
| $view = TestHelper::createView(); | ||
|
|
||
| $this->expectException(InvalidArgumentException::class); | ||
| $this->expectExceptionMessage( | ||
| 'Render stdClass is not an instance of ' . TemplateRendererInterface::class . '.' | ||
| ); | ||
|
|
||
| $view->withRenderers([ | ||
| 'php' => new stdClass(), | ||
| ]); | ||
| } |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test only covers the case where an object that doesn't implement TemplateRendererInterface is passed (stdClass). Consider adding test cases for non-object values such as null, strings, arrays, or scalar values to ensure the validation handles these cases correctly and provides meaningful error messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: samdark <47294+samdark@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.