WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 2 weeks ago

#13867 assigned enhancement

New filter for comment RSS feed's title

Reported by: shidouhikari Owned by: killua99
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Feeds Keywords: needs-refresh good-first-bug has-patch
Focuses: Cc:

Description

I'd like to be able to customize comments titles in RSS feed.

Currently it's hardcoded and has no way to be changed, so I added 2 new filters so that plugins can edit them.

I've tested and patch is working for me.

Attachments (4)

feed-rss2-comments.php.diff (1.9 KB) - added by shidouhikari 8 years ago.
Adds 2 filtters to allow plugins to edit comments titles in RSS feed
13867.diff (2.6 KB) - added by DrewAPicture 3 years ago.
+ hook docs
13867.2.diff (2.6 KB) - added by stevenkword 3 years ago.
updates hook doc
13867.3.patch (2.8 KB) - added by killua99 7 months ago.

Download all attachments as: .zip

Change History (21)

@shidouhikari
8 years ago

Adds 2 filtters to allow plugins to edit comments titles in RSS feed

#1 @nacin
8 years ago

  • Keywords early removed
  • Milestone changed from 3.1 to Future Release

These can already be filtered through gettext filters.

#2 @shidouhikari
8 years ago

  • Cc shidouhikari added

As we talked in IRC, using gettext one would need to test for 'Comment on %1$s by %2$s' and 'By: %s'...

'By: %s' isn't something very exact for title filtering. A specific filter is simpler to handle and has no risk of other 'By: %s' somewhere else being messed. Also, if in a future version one of those texts are changed, anything relying on them would get broken.

With a specific filter, just let core do its work and then filter the result.

And sorry about the early, I thought it means patches in an early new milestone :p

#3 @nacin
8 years ago

  • Milestone changed from Future Release to Awaiting Triage
  • Type changed from feature request to enhancement

Per IRC.

#4 @nacin
7 years ago

  • Milestone changed from Awaiting Triage to Future Release

#5 @chriscct7
3 years ago

Patch still looks good

#6 @DrewAPicture
3 years ago

  • Keywords needs-docs needs-patch added; has-patch removed

A few things here:

  • Both filters could probably use better names
  • Both would need hook documentation for consideration
  • As we need to make sure whatever output is passed through ent2ncr() it might be better to do it just-in-time when the filter value is output, and remove the others from the existing sprintf lines.

@DrewAPicture
3 years ago

+ hook docs

#7 follow-up: @DrewAPicture
3 years ago

  • Keywords has-patch added; needs-docs needs-patch removed
  • Milestone changed from Future Release to 4.2

Seems like with all of the RSS filters we already have, we'd have one or two in place we could reuse, but I'm not really seeing viable alternatives to the ones proposed in the patch.

13867.diff refreshes the 5-year-old patch and adds hook docs. Moving to 4.2 for consideration.

#8 in reply to: ↑ 7 ; follow-up: @stevenkword
3 years ago

Drew, I'm taking a serious look at the way we are applying filters to RSS. I'm finding that we are using too many catchall functions, and I think we need to re-examine the XML and RSS syntax rules. I'm currently putting together a proposal that relates to this ticket.

I'm currently thinking we need to have filters for individual encodings such as proposed in #19998, but I want to hold further comment until I have my head better wrapped around the big picture.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#9 in reply to: ↑ 8 @mdgl
3 years ago

Replying to stevenkword:

Drew, I'm taking a serious look at the way we are applying filters to RSS. I'm finding that we are using too many catchall functions, and I think we need to re-examine the XML and RSS syntax rules. I'm currently putting together a proposal that relates to this ticket.

I agree our whole approach to encoding XML/RSS could do with a bit of a re-vamp, probably on a new ticket. It's actually quite a tricky problem and for a long time we have tended just to throw CDATA blocks around until things appear to work.

See #3670 for the beginnings of some ideas about a more general esc_xml() abstraction. Also related #28816.

This ticket was mentioned in Slack in #core by drew. View the logs.


3 years ago

#11 @ocean90
3 years ago

  • Milestone changed from 4.2 to Future Release

What would be an example use-case for this filter?

#12 @stevenkword
3 years ago

@ocean90 -- Changing the feed title to something that doesn't match the existing phrasing. e.g.) "Comments on: Hello World!" to something like "What People are Saying about Hello World!"

13867.diff still applies cleanly and 13867.2.diff updates the hook doc. I think this is ready to go.

@stevenkword
3 years ago

updates hook doc

#13 @stevenkword
13 months ago

  • Keywords needs-refresh added; has-patch removed

Patch no longer applies cleanly.

#14 @stevenkword
7 months ago

  • Keywords good-first-bug added

@killua99
7 months ago

#15 @killua99
7 months ago

Updating the code till the latest trunk

#16 @dingo_bastard
6 months ago

  • Keywords has-patch added

#17 @DrewAPicture
2 weeks ago

  • Owner set to killua99
  • Status changed from new to assigned

Looks like the attached patch here will need another refresh. Assigning this good-first-bug ticket to mark it as "claimed".

Note: See TracTickets for help on using tickets.