[Feature request] category-RSS when browsing category

[costela]

Hey there,

It occurred to me that when the user is browsing a specific category, he might expect the RSS provided by that page to also point to a category-specific feed.
I thought it might be a feature worth adding. What are the opinions here in the forum about this?

Then comes the other problem: if it's deemed a worthy addition, what's the best way of implementing this?
Changing the current theme to behave this way is simple enough, albeit a bit dirty, but I wanted something cleaner that could interact better with URL-rewriting and that could be switched on and off. Perhaps as a plugin, but most likely as an internal option. Something like "RSS Feeds Follow Current Category".

I imagine adding this functionality to the default index.tpl would already be a big help, but adding

Code: Select all

?category={$category}

to the RSS and Atom links is - although it works - just too dirty and not configurable.
Any pointers on the best practices when trying to introduce such general features?

[garvinhicking]

Hi!

The best practice would be to change the config.inc.php (or create it) of your template. In that you can check if $serendipity['GET']['category'] is set, and then fetch the corresponding RSS feed for that:

Code: Select all

if ($serendipity['GET']['category']) {
  $cat = serendipity_fetchCategoryInfo($serendipity['GET']['category']);
  $catfeed = serendipity_feedCategoryURL($cat, 'serendipityHTTPPath');
  $serendipity['smarty']->assign('catfeed', $catfeed);
}

Then, anywhere in your index.tpl, you can show {$catfeed} for the current RSS feed.

However I personally think it's clearer to the user if you show the "Categories" sidebar plugin with the XML-Button for its feeds so that people can fetch it. Automatically using the current category XML feed inside the syndication plugin's output might IMHO confuse users, because IMHO this should always leed to the full feeed...

But using your custom variant fit to your template, that should be a good way.

HTH,
Garvin

[costela]

Hi,

Thanks for the pointers!

Would you accept a patch turning the RSS/Atom links in the default template into Smarty variables?
This way I could create a plugin to make the RSS links follow the category and make it available through Spartacus. Perhaps someone else finds this functionality useful! And this seems IMHO like a nice step anyway, turning one more aspect of Serendipity pluggable.


BTW, on a completely off-topic note: I just noticed you also live in Kölle. Hey there neighbor!

Cheers
Leo

[garvinhicking]

Hi!

Sure, why not, I think this patch could be added!

BTW, on a completely off-topic note: I just noticed you also live in Kölle. Hey there neighbor!

Nice to hear that. We could meet up to discuss the patch, if you like

Best regards,
Garvin

[costela]

Hi,

So, finally got around to writing that patch. Since it's kindda big (292 lines), should I post it here, in a new thread in the Development forum or send via email?
There are probably a few places where your input would be appreciated.

Nice to hear that. We could meet up to discuss the patch, if you like

If you meant the offer, I'm up for it!
Though you may need a little patience with my German, which is far from perfect....

Cheers
Leo

[garvinhicking]

Hi!

The best way would be to post it on a service like http://nopaste.php-q.net/ and post your link here? That way I can easily download the patch -- it's actually not so easy for me to download a mail attachment and re-upload it to my server

Then I can gladly try to discuss the remaining questions here

If you meant the offer, I'm up for it!
Though you may need a little patience with my German, which is far from perfect....

Not a problem. What is you native language? I simply assumed it would be german, but your nick sounds...hm, spanish?

If you want, you can reply via PM so that we can find a meeting point here in cologne...

Regards,
Garvin

[costela]

Hi,

I put it on http://yourpaste.net/868/ because they have diff syntax highlighting and line numbering.

This patch was made on 1.3.1, but applies cleanly to a SVN checkout made yesterday.

Some (many?) comments on the patch:

• The changes in compat.inc.php shouldn't probably be there, but I couldn't find a better place to put it. I didn't want to clutter the config screen with more stuff, but at the same time feel it's still very hacky to take something hardcoded from the template and hardcode it on the PHP. So what's your suggestion for a place to put this?
• The changes in functions.inc.php are necessary to enable standardized names for feeds. It should be totally backwards compatible, but enable us to use generic file names with extension based feedformat detection.
• All the changes in tpl/config_local.inc.php and most of them in functions_permalinks.inc.php are also related to the above change, with the exceptions:
  • The regular expressions now use named captures, so the order of the substitutions is not important. Before this change, if a user changed %id%-%name% to %name%-%id% the code would not use the ID. This is something of a bugfix, but it was also needed for the rest of the patch. I've seen no problems with Apache's mod_rewrite, but this might need some more testing. (I haven't made this change to the rest of the code to avoid an over-reaching patch, but if you like the idea, I could give it a try)
  • Bugfix in function serendipity_searchPermalink(): as commented in the code, the permalinks table has only urls for normal entries, so if the format for the feed's URL is different, it wouldn't be found. This is a solution which I'm also not sure fits s9y's coding mentality... it's at least backward compatible, but doesn't feel quite right.
• The changes in index.php, aside from being a small performance boost (removing a preg_match and two strpos), also make the feed code behave more consistently. It makes use of the serendipity_searchPermalink changes mentioned above and allows us to use a standardized name for the feeds.

Well, let me know what you think.

Not a problem. What is you native language? I simply assumed it would be german, but your nick sounds...hm, spanish?

Close guess! My native language is Portuguese (specifically pt_BR), but I gotta train my German, so if it doesn't strain your ears too much I'd be glad to give it a try!

PMing about a meeting point.

Cheers
Leo

[garvinhicking]

Hi!

I've just had a quick look over the diff. It looks quite good, the only thing I'm afraid of are:

1. that named captures might not work in all PHP versions? Do you know since which PCRE this became available? And as for mod_rewrite, does it work for both Apache1 and Apache2?

2. The version additions to the URL, do they make trouble when people call them without a version? Like, does index.rss properly work to display the RSS2 feed, or would it display RSS1, or neither of them?

I haven't yet had the chance to apply the patch, I hope to be able to do that soon.

Personally, I'd rather move the code parts from currently compat.inc.php to the permalinks function file, in the initPermalink*() function?

Regards,
Garvin

[costela]

1. that named captures might not work in all PHP versions? Do you know since which PCRE this became available? And as for mod_rewrite, does it work for both Apache1 and Apache2?

Tested on PHP5 (5.2.5) and PHP4 (4.4.4).
Unfortunately Apache 1.3.34 doesn't seem to support it. They could be parsed out of the code while generating the .htaccess, this way the advantages of the named captures remain without impacting Apache.

2. The version additions to the URL, do they make trouble when people call them without a version? Like, does index.rss properly work to display the RSS2 feed, or would it display RSS1, or neither of them?

It's backward compatible, so a file ending in "rss" displays a RSS1.0 feed. The default links being used ended all in "rss2". This is - of course - very easy to change, but I thought preserving the current behavior was preferable.

Personally, I'd rather move the code parts from currently compat.inc.php to the permalinks function file, in the initPermalink*() function?

Fully agreed.

Cheers
Leo

[garvinhicking]

Hi!

Tested on PHP5 (5.2.5) and PHP4 (4.4.4).

Good to know!

Unfortunately Apache 1.3.34 doesn't seem to support it. They could be parsed out of the code while generating the .htaccess, this way the advantages of the named captures remain without impacting Apache.

Hm, it might get hard to parse them out for Apache 1 only...better to remove it completely for .htaccess, is that possible? I haven't yet properly read the code to understand why named captures are required, though

It's backward compatible, so a file ending in "rss" displays a RSS1.0 feed. The default links being used ended all in "rss2". This is - of course - very easy to change, but I thought preserving the current behavior was preferable.

Oh, I always thought the index.rss feed was RSS2 (like index.rss2) and RSS1 only was index.rss1?

Regards,
Garvin

[costela]

Hi!

Sorry for the delayed answer, the week has been a bit of a chaotic mess.

Hm, it might get hard to parse them out for Apache 1 only...better to remove it completely for .htaccess, is that possible? I haven't yet properly read the code to understand why named captures are required, though

Taking it out of the htacces is just a matter or parsing it out in include/functions_installer.inc.php:.
As for the reason, AFAICT it's not strictly necessary, but it helps code readability by the complex regexps and enables users to really manipulate the %id%-%name% strings, including changing the order of the elements. Until now, if the user had %name%-%id%, serendipity wouldn't treat the id as a special field and instead would do a search in the permalinks table to find the category, for instance.

Oh, I always thought the index.rss feed was RSS2 (like index.rss2) and RSS1 only was index.rss1?

Taking a look at the 2nd block from the patch I stand corrected: include/functions.inc.php says that the urls ending with just "rss" are actually treated as RSS 0.91.

Cheers
Leo