Conversation
Added a method to allow custom match types for AltoRouter.
Added documentation for the addMatchTypes method in README.md.
There was a problem hiding this comment.
Pull request overview
This PR adds a public wrapper API to expose AltoRouter custom match-type registration through the Routes WordPress plugin, along with README documentation for using it.
Changes:
- Added
Routes::addMatchTypes()as a wrapper around AltoRouter’saddMatchTypes(). - Documented
addMatchTypesusage inREADME.mdwith an example.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| Routes.php | Introduces a new addMatchTypes wrapper method and related PHPDoc. |
| README.md | Adds a new documentation section describing how to register custom match types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Routes.php
Outdated
| public static function addMatchTypes($match_types) | ||
| { | ||
| global $upstatement_routes; | ||
| if (isset($upstatement_routes->router)) { | ||
| $upstatement_routes->router->addMatchTypes($match_types); | ||
| } |
There was a problem hiding this comment.
addMatchTypes() currently does nothing if it’s called before the first Routes::map(), because $upstatement_routes->router is only instantiated inside map(). The README example calls addMatchTypes() before map(), so custom match types won’t be applied. Consider initializing the router here when it’s missing (ideally by extracting the router setup logic from map() into a shared helper) or persisting pending match types and applying them when the router is created.
Routes.php
Outdated
| public static function addMatchTypes($match_types) | ||
| { |
There was a problem hiding this comment.
Method brace placement is inconsistent with the rest of this file (e.g., map() uses { on the same line as the function signature). For consistency/readability, match the existing style here.
| public static function addMatchTypes($match_types) | |
| { | |
| public static function addMatchTypes($match_types) { |
Routes.php
Outdated
| public static function addMatchTypes($match_types) | ||
| { | ||
| global $upstatement_routes; | ||
| if (isset($upstatement_routes->router)) { | ||
| $upstatement_routes->router->addMatchTypes($match_types); | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add PHPUnit coverage for Routes::addMatchTypes() to ensure custom match types are applied as expected (especially for the documented call order of addMatchTypes() before map()). This will prevent regressions in router initialization / match-type registration behavior.
README.md
Outdated
| Send an optional status code. Defaults to 200 for 'Success/OK' | ||
|
|
||
| ## addMatchTypes | ||
| This method makes it possbile to add custom matchtypes in Routes. |
There was a problem hiding this comment.
Typo in README: “possbile” should be “possible”.
| This method makes it possbile to add custom matchtypes in Routes. | |
| This method makes it possible to add custom matchtypes in Routes. |
README.md
Outdated
| Routes::addMatchTypes([ | ||
| 'oldID' => '@[0-9]++', | ||
| ]); | ||
|
|
There was a problem hiding this comment.
The README example calls Routes::addMatchTypes() before Routes::map(), but the current implementation only forwards match types if the router has already been created. Either update the example to reflect the required call order, or (preferably) update addMatchTypes() to initialize/apply match types even when called before any routes are mapped.
|
Thanks for the PR @Levdbas ! Copilot found a few documentation nit-picks that are worth resolving. The biggest Q though is this finding ...
Is there a timing/initialization issue here? Thanks! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…match types functionality
…-altorouter-addMatchTypes
…ter-addMatchTypes
…ch_current_request method
Added a method to allow custom match types for AltoRouter.