Make WordPress Core

Opened 18 years ago

Closed 17 years ago

#2433 closed defect (bug) (fixed)

Provide some API for WP_Rewrite

Reported by: davidhouse's profile davidhouse Owned by:
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.0.1
Component: Administration Keywords: has-patch 2nd-opinion dev-feedback
Focuses: Cc:

Description (last modified by davidhouse)

As discussed http://comox.textdrive.com/pipermail/wp-hackers/2006-February/004528.html

This patch adds add_rewrite_rule(), add_rewrite_tag(), add_feed(). add_endpoint() and add_base() (if we want to include that one) are still needed.

Specification for these functions: http://codex.wordpress.org/User%3ADavidHouse/WP_Rewrite_API

By the way, I haven't had a chance to test this yet, this isn't meant to be 100% functional code, just some implementation ideas. Feel free to poke at it. I'll probably test and bugfix tomorrow.

Release early and often, right? :)

Attachments (7)

wp-rewrite-api.diff (4.1 KB) - added by davidhouse 18 years ago.
wp-rewrite-api.2.diff (14.4 KB) - added by davidhouse 18 years ago.
wp-rewrite-api.3.diff (19.5 KB) - added by davidhouse 18 years ago.
wp-rewrite-api.4.diff (45.3 KB) - added by davidhouse 18 years ago.
wp-rewrite-api.5.diff (45.1 KB) - added by davidhouse 18 years ago.
Minus a deadly print_r
wp-rewrite-api.6.diff (45.1 KB) - added by davidhouse 18 years ago.
Cleanup, forgot to rename the function calls
wp-rewrite-api.7.diff (46.0 KB) - added by davidhouse 18 years ago.

Download all attachments as: .zip

Change History (36)

#1 @davidhouse
18 years ago

  • Description modified (diff)

#2 @ryan
18 years ago

Sounding good. I'd like to have add_endpoint() and probably add_base() too. Also, we should use as much of this API as we can within WP_Rewrite itself. Trackback would be just another endpoint added by add_endpoint(), for example.

#3 @davidhouse
18 years ago

Pass two. Just a bugfix for my previous patch, I'm yet to add add_endpoint() and add_feed(). Issues raised:

  • How to handle rewrite rules that won't redirect to index.php (i.e., general purpose rewrite rules that don't involve WP)? I've taken the view that WP_Rewrite should be a general-purpose rewrite rules handler (it's only a few lines of code away from being one anyway) that should always be used when adding rewrite rules to .htaccess, so I've written a few lines to deal with these types of rules. They're stored in WP_Rewrite::non_wp_rules and in a loop in mod_rewrite_rules() outputs them before the rest of WP's rules (note that if you were using the verbose rules before then these types of rules were already handled).
  • During debugging I had to plough through generate_rewrite_rules and I decided enough was enough, so I've gone through and added comments to nearly every line.

#4 @ryan
18 years ago

index.php can handle anything directed to it. Just use the template_redirect action coupled with a query var.

function myplugin_template_redirect() {
        // Get tmy_var from WP_Query.
        $my_var = get_query_var('my_var');

        // If it's empty, return.
        if (empty($my_var))
                return;

        // Otherwise, load our template.
        // You must use load_template rather than require or include.
        // load_template() sets up the necessary environment.
        load_template(ABSPATH . 'my_handler.php');
        // Make sure you exit after loading a template.
        exit;
}

add_action('template_redirect', 'myplugin_template_redirect');


#5 @davidhouse
18 years ago

What if you want to redirect to a text file? Will WP return a mimetype of text/plain? What about an image? How are you going to include a binary file?

#6 @ryan
18 years ago

Good point. non_wp_rules sounds good for those cases then.

#7 @davidhouse
18 years ago

I'm about to start writing add_endpoint(), and I need an opinion. Westi (who wrote the spec for this function) said:

"This simply adds the endpoint to all permalink types (e.g. posts, pages, category, author, search) and then template-loader.php includes the relavent handler file"

The problem is that we should probably allow plugins to specify onto which URLs their endpoint gets added on to. E.g. trackback needs to be added just on to permalinks, but others might want their endpoint only on, say, date URLs. Do we:

  1. Just give an option for permalinks/everywhere
  2. Give a whole list of bool parameters for date/category/author/etc (perhaps wrapped up in some sort of array or something)

#8 @majelbstoat
18 years ago

The case exists where a plugin could want to apply a different string to the ends of different kinds of links, so an array would be nice.

I think PHP can do bitwise OR of flags? Something like

add_end_point('myfunction1', DATE_LINK
CATEGORY_LINK AUTHOR_LINK);

add_end_point('myfunction2', PERMALINK);

would be flexible.

#9 @davidhouse
18 years ago

I like the bitflags idea. If plugins want to add different strings to different permalinks then they can just do more than one add_endpoint() call.

#10 @majelbstoat
18 years ago

Yeah, that was my thinking. I'm encouraged by all the work you're doing on this and look forward to the results,

Cheers,

Jamie.

#11 @majelbstoat
18 years ago

I've just realised that Trac stripped the pipe operator from between those flags in my first comment (but I think you guessed that).

There would have to be some sort of decoding mechanism, but that wouldn't be to hard to implement.

define (DAY_LINK, 1);
define (MONTH_LINK, 2);
define (YEAR_LINK, 4);

function add_endpoint($function_name, $flags)
{

if ($flags & DAY_LINK) add_filter('day_link', $function_name);

}

or something more extensible:

$links = array (DAY_LINK => 'day_link', YEAR_LINK => 'year_link');

then

function add_endpoint($function_name, $flags)
{

global $links

foreach ($links as $flag => $filter) if ($flags & $flag) add_filter($filter, $function_name);

}

#12 @davidhouse
18 years ago

Here's add_endpoint(). I haven't converted trackback over to using said function yet because trackback URLs are required after, say <permalink>/attachment/ URLs, and the best add_endpoint() currently offers is to put your endpoint after a permalink.

Similarly to add_rewrite_tag(), you don't pass any callbacks to add_endpoint(). The function sets up the rewrite rules for you, then you add your own filters onto whichever hooks you please.

The constants available are EP_PERMALINK, EP_DATE, EP_ROOT, EP_COMMENTS, EP_SEARCH, EP_CATEGORIES, EP_AUTHORS, EP_PAGES. There are also two extra constants available for convenience, EP_NONE and EP_ALL.

#13 @majelbstoat
18 years ago

I'm unclear as to the details, but does this allow adding an endpoint to just a month, or just a year?

As in site.com/2006/02/16/myendpoint/ is handled, but what about site.com/2006/02/myendpoint/ and site.com/2006/myendpoint/

If it doesn't could we take this opportunity to split up the date rules into year month and day rules allowing for this to happen?

If these cases are already handled, happily ignore this :)

Also, why do we need $feeds *and* $feed_files? It seems redundant to store the same data twice when you could easily see if the feed type exists by looking at the keys to $feed_files.

#14 @davidhouse
18 years ago

No, the most specific you can get at the minute is EP_DATE.

$feed_files is needed to get the filename for a given feed. See WP_Rewrite::add_feed(), and wp-feed.php.

#15 @majelbstoat
18 years ago

Yes, $feeds is redundant, not $feed_files.

I take it more granularity of endpoints is in the works?

#16 @davidhouse
18 years ago

$feeds is generally relied on by different parts of WP_Rewrite, so I wanted to leave it in. Also, the most elegant way of having a feed files array would be an associative array, names => filenames. If I wanted to get rid of $feeds, then all over the place in WP_Rewrite people would have to do array_keys($this->feeds) which would get ugly. So I split it.

I guess more granularity can be coded, yes. If we want to support trackback through add_endpoint() then it'll be necessary. It will require something of a rethink, though.

#17 @majelbstoat
18 years ago

Take your point. How about doing it the other way and having the filename stored as the keys in $feeds then?

as in $feeds = array ('wprss2.php' => 'rss2', ...);

that wouldn't require any changes and would still limit the redundancy. You may still feel this too much, though, which we could agree to disagree on :)

I really think granularity is of key importance here. There's no point providing an API that doesn't allow full flexibility, as we'd only end up having to code around it in the end anyway. An example I specifically want to have work easily is adding a 2 letter language code to the end of every link. This includes month and year archives. At the moment, it can be done anyway, but it would be nice to use the new API once it exists...

#18 @davidhouse
18 years ago

Then we'd have to do an array_search() in wp-feed.php, which again is ugliness. There's no harm in maintaining two lists when all additions go through WP_Rewrite::add_feed.

Something I just thought of: it might be useful to have delete_feed() and change_feed_file() as well.

#19 @ryan
18 years ago

Some suggestions, lets use callbacks instead of files for add_feed(). Change wp-feed.php to kick the callback for the requested feed. The callback can load a file or generate the feed itself.

Rename add_extra_qv() to add_query_var().

Rename add_extra_rule() to add_rule().

Rename add_non_wp_rule() to add_external_rule().

Maybe put WP_Rewrite and thee add_rewrite() wrapper functions in rewrite.php.

Would adding an optional argument to add_rewrite_tag() that installs a template_redirect callback be useful? Might not be enough added convenience to be worthwhile.

#20 @davidhouse
18 years ago

Nice ideas. Sadly I'll won't have access to my laptop this week: just brief access to Trac at school (and I don't fancy doing development with notepad ;)). I'll try to begin work again this coming weekend, incorporating the changes that Ryan suggested, adding the extra granularity, and who knows, I might even have time to have a crack at add_base() :)

#21 @davidhouse
18 years ago

Bit of a monster of a patch this time. This just moves the WP_Rewrite class (with the API) to rewrite.php, and takes care of the function renames that Ryan suggested.

@davidhouse
18 years ago

Minus a deadly print_r

@davidhouse
18 years ago

Cleanup, forgot to rename the function calls

#22 @davidhouse
18 years ago

Here's a patch which clears up an endpoint bug and adds some granularity: you can now use EP_DAY, EP_MONTH or EP_YEAR to limit to a specific type of date URL, and you can use EP_ATTACHMENT to get your endpoint added on to all the attachment URLs (for trackback).

I haven't moved trackback over to the API yet but it should now be possible.

#23 @ryan
18 years ago

[3638]

I changed feeds to use actions. add_feed() returns a hook name which you can then associate with a function with add_action(). Whaddya think?

#24 @majelbstoat
18 years ago

The improvements here seem great - good work! Couple of questions though:

Is there a clean way to add a regex endpoint that can have multiple possibilities?

Like /(en|fr|de|ja)/ being handled. As I can see it, add_endpoint('/(en|fr|de|ja)/', EP_ALL) would add that regex to the end of each url, but then adds a query var of the same regex. I'd want it to add a query_var of 'language' I'm looking for something like add_rewrite_tag($tag, $pattern, $query) but for endpoints, like:

add_endpoint($pattern, $places, $query=) instead of just add_endpoint($name, $places)

then:

add_endpoint_pattern('(en|fr|de|ja)', EP_ALL, 'language');

That shouldn't be too hard to implement should it? If $query is specified, use that as the query_var. If it remains empty, just use the pattern as now.

Also, what if I wanted to add an endpoint after another endpoint? Am I still looking at using the rewrite_rules_array() filter?

Finally, what does add_query_var() actually do? It adds to an array public_query_vars, but I can't find where that is being used?

#25 @davidhouse
18 years ago

All the QVs mentioned in WP::public_query_vars are checked in WP::parse_query.

I'll look into the endpoint stuff.

#26 @matt
17 years ago

  • Milestone changed from 2.1 to 2.2

#27 @moronicbajebus
17 years ago

First of all thank you for adding this feature. I have been testing the add_endpoint with a plugin and I think I may have found a small bug.

A call to get_query_var($name) returns empty. I am assuming the issue lies with the generated query for it is "&$name=". When I simply changed the generated query to "&$name=1" I had no issue.

#28 @foolswisdom
17 years ago

  • Keywords has-patch 2nd-opinion dev-feedback added; bg|has-patch bg|2nd-opinion bg|dev-feedback removed
  • Milestone changed from 2.2 to 2.3

#29 @ryan
17 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Calling this one done. Use separate tickets for further bugs.

Note: See TracTickets for help on using tickets.