Skip to content

Implement addMatchTypes method#50

Open
Levdbas wants to merge 14 commits intomasterfrom
Levdbas-add-altorouter-addMatchTypes
Open

Implement addMatchTypes method#50
Levdbas wants to merge 14 commits intomasterfrom
Levdbas-add-altorouter-addMatchTypes

Conversation

@Levdbas
Copy link
Copy Markdown
Collaborator

@Levdbas Levdbas commented Mar 23, 2026

Added a method to allow custom match types for AltoRouter.

Added a method to allow custom match types for AltoRouter.
@Levdbas Levdbas changed the title Implement addMatchTypes method in Routes.php Implement addMatchTypes method Mar 23, 2026
Added documentation for the addMatchTypes method in README.md.
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 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’s addMatchTypes().
  • Documented addMatchTypes usage in README.md with 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
Comment on lines +85 to +90
public static function addMatchTypes($match_types)
{
global $upstatement_routes;
if (isset($upstatement_routes->router)) {
$upstatement_routes->router->addMatchTypes($match_types);
}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Routes.php Outdated
Comment on lines +85 to +86
public static function addMatchTypes($match_types)
{
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
public static function addMatchTypes($match_types)
{
public static function addMatchTypes($match_types) {

Copilot uses AI. Check for mistakes.
Routes.php Outdated
Comment on lines +85 to +91
public static function addMatchTypes($match_types)
{
global $upstatement_routes;
if (isset($upstatement_routes->router)) {
$upstatement_routes->router->addMatchTypes($match_types);
}
}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
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.
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in README: “possbile” should be “possible”.

Suggested change
This method makes it possbile to add custom matchtypes in Routes.
This method makes it possible to add custom matchtypes in Routes.

Copilot uses AI. Check for mistakes.
README.md Outdated
Comment on lines +136 to +139
Routes::addMatchTypes([
'oldID' => '@[0-9]++',
]);

Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@jarednova
Copy link
Copy Markdown
Member

Thanks for the PR @Levdbas ! Copilot found a few documentation nit-picks that are worth resolving. The biggest Q though is this finding ...

#50 (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.

Is there a timing/initialization issue here? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants