Make WordPress Core

Opened 17 years ago

Closed 10 years ago

Last modified 10 years ago

#7092 closed enhancement (duplicate)

Should support separate setting for limiting comments feed

Reported by: redsweater's profile redsweater Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: Feeds Keywords: needs-refresh
Focuses: Cc:

Description

Right now the comments feeds are throttled by the same limiting number the user chooses for "posts feeds" (posts_per_rss).

It makes sense that a blog owner would want to set a higher threshold for comments than for posts, for these reasons:

  • Readers who subscribe to comment feeds are fewer in number and less likely to burden the system in aggregate.
  • Comments are typically shorter per entry than posts are.
  • Comments are liable to expand much more quickly than posts, making it difficult for readers to "keep up" without wrapping past the limit.

For instance in my blog I limit post feeds to 10 entries, but I'd like to offer a much larger limit for comments subscribers.

Establishing a "comments_per_feed" variable that is used in place of the "posts_per_rss" would accomplish this.

Attachments (2)

hakre-commentsfeedlength-7092.php (2.6 KB) - added by hakre 15 years ago.
Have fun / v0.1.2
7092.diff (6.5 KB) - added by nacin 15 years ago.

Download all attachments as: .zip

Change History (33)

#1 @ryan
16 years ago

  • Milestone changed from 2.7 to 2.8

#2 @FFEMTcJ
16 years ago

  • Keywords needs-patch added
  • Milestone changed from 2.8 to Future Release

#3 @Denis-de-Bernardy
16 years ago

  • Component changed from General to Feeds
  • Milestone Future Release deleted
  • Resolution set to invalid
  • Status changed from new to closed

plugin material

#4 @redsweater
16 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I'm reopening this unless a more substantial explanation for closing it is provided. "plugin material" doesn't mean much to me, and doesn't serve as a valuable record of the dismissal process.

Thanks for considering keeping this as a legitimate enhancement request. I think it would benefit the project to eventually adopt the changes I recommended.

If the shorthand used in closing this bug was representative of a more thoughtful elimination process, I think it would serve the project to append the actual reasons for dismissing and closing this bug.

Thanks!
Daniel

#5 @Denis-de-Bernardy
16 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

As a rule of thumb, the UI mindset is less options = better. The best options without asking the user = best.

This would add new options. There already are two options related to this, which should probably be dropped instead.

There are a few plugins that let you change the number of posts based on the type of archive. One could be used for inspiration to make one that changes the number of comments in feeds.

#6 @Denis-de-Bernardy
16 years ago

Just read your comment in the wordpress-dev chat log. I'll do my best to immediately be more verbose next time. :-)

#7 @lapcat
15 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Please reconsider implementing this. I've hacked my own WordPress blog to fix it, but the problem keeps hitting me when I subscribe to the comments feeds of other people's blogs, and they don't even realize there's a problem. If you have a 'hot' post where there are a lot of comments, you miss a bunch of them in the feed, because most people have their setting at the default 10.

This is not plugin material. The basic functionality of WordPress in this area is broken. It's a very frequent occurrence that a new post gets 20-30 comments in a day or two, but it's a much less frequent occurrence that there are 20-30 posts in a day or two, so the two feeds should have differenent settings.

If you don't want to have a new option in the user interface, then just change the default for the comments feed to something higher, like 50 or 100.

#8 @scribu
15 years ago

lapcat, could you demonstrate how you hacked your WordPress, possibly in the form of a patch?

#9 @lapcat
15 years ago

scribu, here's the little hack I did in wp-includes/query.php:

-				$climits = apply_filters('comment_feed_limits', 'LIMIT ' . get_option('posts_per_rss'));
+				$climits = apply_filters('comment_feed_limits', 'LIMIT 100');

-			$climits = apply_filters('comment_feed_limits', 'LIMIT ' . get_option('posts_per_rss'));
+			$climits = apply_filters('comment_feed_limits', 'LIMIT 100');

#10 @dd32
15 years ago

  • Keywords dev-feedback added
  • Milestone set to Future Release
  • Version set to 2.9

Just setting this to a Future release pending developer feedback..

As for that hack, you can replace it with a single line in your themes functions.php file:

add_filter('comment_feed_limits', create_function('$a', 'return "LIMIT 100";'));

#11 @hakre
15 years ago

I think this report is very valid, good point. Even though it's simple to hack (as most stuff is), I would like to see this in because it really makes sense.

#12 @hakre
15 years ago

As it is now common sense that this is plugin material I added a plugin for reference and to show how easy this is to implement. Save the file in your plugin folder and activate the commentsfeedlength plugin. Then visit Settings -> Reading and you'll find the setting at the end of the reading options default section. Just enter the value you see fit.

I suggest to close as invalid since this really is plugin material.

#13 @hakre
15 years ago

  • Keywords needs-patch dev-feedback removed

@hakre
15 years ago

Have fun / v0.1.2

#14 @hakre
15 years ago

Related: #7682

#15 follow-up: @redsweater
15 years ago

What makes it "common sense" that this is plugin material?

Should the setting for controlling how many posts are in the RSS Feed be eliminated and moved to a plugin?

Please provide us with a little more information about your decision - the common sense you allude to is not self-evident. My best interpretation of what you are saying is that because it is possible to implement the feature as a plugin, that is necessarily where it should be done.

I would tend to agree with lapcat's assessment above: "The basic functionality of WordPress in this area is broken." Generally speaking, fundamentally broken things should be fixed in the product, not in a plugin.

#16 in reply to: ↑ 15 @hakre
15 years ago

  • Keywords needs-patch added

Replying to redsweater:

...

You do not need to use the plugin. Feel free to provide a patch for the core so that it can be reviewed.

#17 @redsweater
15 years ago

I reported this bug as a user of WordPress. I probably will not get around to creating a patch for it, in case that affects the willingness of the WordPress team to keep the bug open.

#18 @hakre
15 years ago

Can you actually provide some feedback if you use the provided plugin it does the job? Just asking, no actual need for you to use it generally just to find out if that code does what your intention is.

#19 @nacin
15 years ago

I may just patch this, that way a dev can do a thumbs up or down on it once and for all.

#20 @nacin
15 years ago

  • Keywords has-patch added; needs-patch removed

We already have posts_per_page, comments_per_page, posts_per_rss. Patch (untested) adds comments_per_rss. As much as we hate options in core, I think it does make sense and easily fits into the options-reading UI.

I chose for it to follow the footsteps of posts_per_rss, instead of comments_per_page -- it currently relies on posts_per_rss anyway.

It would default to whatever posts_per_rss is.

@nacin
15 years ago

#21 follow-up: @redsweater
15 years ago

Hi nacin - I installed and tested your patch and it seems to do what I was requesting. Thank you!

One question: I notice you added something to schema.php that appears to incorporate the posts_per_rss as the default value for comments_per_rss. When does this get applied? Is this something that will get automatically set up when a database upgrade occurs?

I noticed after installing the patch that when I went to the Reading preferences, the value was blank. So I want to make that there is a plan to have a default value applied when the feature reaches real end-users.

#22 in reply to: ↑ 21 @nacin
15 years ago

Replying to redsweater:

Hi nacin - I installed and tested your patch and it seems to do what I was requesting. Thank you!

One question: I notice you added something to schema.php that appears to incorporate the posts_per_rss as the default value for comments_per_rss. When does this get applied? Is this something that will get automatically set up when a database upgrade occurs?

If this gets committed, the core dev making the commit will bump the $wp_db_version global in wp-includes/version.php to the latest revision number. This will force a database upgrade which will run the schema and add the new option.

To test this, delete the comments_per_rss option from your options table, then increase the $wp_db_version global by 1. On load, you'll get an "upgrade required" screen and the default value should be applied.

#23 @nacin
15 years ago

(In [13823]) Syndication feeds show the (#) most recent 'items', not posts, as it applies to comments as well. See #12556, see #7092

#24 @ruijt
15 years ago

  • Cc ruijt added
  • Type changed from enhancement to defect (bug)

Hi nacin... Exactly what I needed...

The only thing is an error on the admin/reading page:

Warning: Missing argument 1 for commentsfeedlengthPlugin::settings_field_input_callback() in /home/admin/domains/*/public_html/site/wp-content/plugins/hakre-commentsfeedlength-7092.php on line 101

Warning: extract() [function.extract]: First argument should be an array in /home/admin/domains/*/public_html/site/wp-content/plugins/hakre-commentsfeedlength-7092.php on line 102

Can you help me with this?

Regards, Bas

#25 @nacin
15 years ago

  • Type changed from defect (bug) to enhancement

hakre can probably answer questions offlist about his plugin. hakre.wordpress.com.

#26 follow-up: @chriscct7
10 years ago

  • Resolution set to duplicate
  • Status changed from reopened to closed

This was fixed in #9635 and further revised in #9886

#27 @DrewAPicture
10 years ago

  • Milestone Future Release deleted

Duplicate of #9635.

#28 in reply to: ↑ 26 ; follow-ups: @redsweater
10 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Replying to chriscct7:

This was fixed in #9635 and further revised in #9886

I don't think either of the cited tickets are suitable duplicates of this one. Trunk WordPress still doesn't support the feature requested by this ticket: separate user control over the number of items in the comments feed.

nacin's earlier patch made the point of establishing a new setting for "comments_per_rss", and added UI to the reading options in the settings admin UI. Neither of those goals is achieved (so far as i can tell) by the tickets cited as duplicates.

Take a look at wp-includes/query.php line 3129 in today's trunk core sources to see that comment_feed_limits is derived from posts_per_rss, and there doesn't appear to be any user setting for overriding that.

It seems that because the patch was never approved, it will probably need to be revised to apply cleanly on modern trunk. I can take a stab at this unless the climate has changed since 5 years ago and it is no longer thought this ticket stands a chance ;)

If it's going to be closed, it should be closed as invalid instead of incorrectly stating that other tickets have rendered it moot.

#29 in reply to: ↑ 28 @chriscct7
10 years ago

  • Keywords needs-refresh added; has-patch removed
  • Resolution set to duplicate
  • Status changed from reopened to closed

Replying to redsweater:

Replying to chriscct7:

This was fixed in #9635 and further revised in #9886

I don't think either of the cited tickets are suitable duplicates of this one. Trunk WordPress still doesn't support the feature requested by this ticket: separate user control over the number of items in the comments feed.

The ticket is requesting a way to modify the number of comments in the comment thread. #9635 adds a filter called comment_feed_limits (later edited in #9886) that allows you to be able to directly manipulate just the number of comments on the comment thread. Nacin's patch doesn't add a UI, nor did the ticket request it, nor does a UI for editing the number of comments on the comment feed make sense because I highly doubt most people would ever want to do this, and for the extreme vast majority that do, a filter is there for it thanks to #9635, so we don't need "yet another setting".

Its important to note, the ticket never asked for a UI setting, it asked for a variable (he meant to say filter) on the last line.

That being said, #9635 solves all objectives stated in the ticket, and thus is a duplicate of that ticket.

#30 in reply to: ↑ 28 @DrewAPicture
10 years ago

Replying to redsweater:

Replying to chriscct7:

This was fixed in #9635 and further revised in #9886

I don't think either of the cited tickets are suitable duplicates of this one. Trunk WordPress still doesn't support the feature requested by this ticket: separate user control over the number of items in the comments feed.

It's effectively a duplicate of #9635 because the spirit of the ticket was met by introducing a filter to allow modifying the comment feed limits, comment_feed_limits.

nacin's earlier patch made the point of establishing a new setting for "comments_per_rss", and added UI to the reading options in the settings admin UI. Neither of those goals is achieved (so far as i can tell) by the tickets cited as duplicates.

An option isn't going to be added per the "decisions not options" mantra. The filter allows plugins to modify the value without adding an additional option, and either of the two examples I just posted on the comment_feed_limits hook reference should suffice.

If you want an option-based solution, check out @hakre's plugin hakre-commentsfeedlength-7092.php attached above.

If it's going to be closed, it should be closed as invalid instead of incorrectly stating that other tickets have rendered it moot.

I'm reclosing it as a duplicate.

#31 @redsweater
10 years ago

I maintain it's not a duplicate and think that all the good reasoning earlier in this ticket's history make the case for it being a desirable built in option.

Thanks for considering it, though! I appreciate that the case is now closed.

Note: See TracTickets for help on using tickets.