Unnecessarily cryptic code when checking pagename= for verbose rules

[duck_]

[18541] for #16687 contains:

if ( $wp_rewrite->use_verbose_page_rules && preg_match( '/pagename=\$([^&\[]+)\[([0-9]+)\]/', $query, $varmatch ) ) { 
    // this is a verbose page match, lets check to be sure about it 
    if ( ! get_page_by_path( ${$varmatch[1]}[$varmatch[2]] ) )

The variable variable is unnecessary as we know we want to be looking in the $matches array.

The regular expression could also be made clearer with /pagename=\$matches\[([0-9]+)\]/. We already assume it is (see above) and if it's not $matches then WP_MatchesMapMatches isn't going to work anyway.

[Otto42]

While you're correct in the context of the core pagename rules, I wrote it this way in order to account for pagename rules that might be added by other systems/plugins. Basically, we don't know that pagename=matches here, not for sure.

Now, the regexp may be off for other cases, but I'd prefer to see some of those other cases before saying that pagename is always going to be =$matches[x].

Maybe I'm being overly cautious?

[Otto42]

That said, this would be a very good optimization, if you want to assume pagename will always = $matches[x]. I was originally thinking of pagename=something-static here, but the regexp won't account for that as written, so it's broken anyway for that case.

[duck_]

Reply got delayed until after your 2nd one, but will post anyway.

Replying to Otto42:

Maybe I'm being overly cautious?

The variable variable doesn't make sense if it's not "matches" as there's no other array to use.

As for the regular expression. If the query is not pagename=$matches[n] then it's not going to be used by WP_MatchesMapRegex anyway since WP_MatchesMapRegex::$_pattern is '(\$matches\[[1-9]+[0-9]*\])'.

I wasn't sure about this at first too. I even followed the code back to [881] to see if a different variable name was ever useful/worked here.

[Otto42]

My first couple of stabs at this were attempting to grab whatever-the-heck was after pagename and use it appropriately, under the consideration that I don't know everything and somebody might have something else there in some weird rewrite rule. This turned out to not work because of the [array] parameter bits. So it sort of evolved into this over time.

If we're sure that $matches[x] is the only thing that will ever work here anyway, then this patch is perfectly fine and should get pushed into 3.3.

[nacin]

Per WP_MatchesMapRegex's $_pattern: '(\$matches\[[1-9]+[0-9]*\])'. Seems like the [1-9]+[0-9]* should also venture into these regexes.

Looks good otherwise. If it's not $matches, it won't work.

[nacin]

In [19017]:

We know that pagename is being assigned to $matches-something, so skip the variable variable. props duck_. see #18991.