Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#37319 closed defect (bug) (fixed)

Documentation for first parameter of `comment_max_links_url` is incorrect

Reported by: ocean90 Owned by: rachelbaker
Milestone: 4.6 Priority: normal
Severity: normal Version: 3.8
Component: Comments Keywords: has-patch
Focuses: docs Cc:


$num_links is not the "number of links allowed", it's the number of found links in a comment.

Attachments (3)

comment.php.13.patch (5.2 KB) - added by pbearne 5 years ago.
updated function
comment.php.14.patch (1.4 KB) - added by pbearne 5 years ago.
comment and new filter that is usefull
comment.php.15.patch (754 bytes) - added by pbearne 5 years ago.
Just comment

Download all attachments as: .zip

Change History (10)

#1 @pbearne
5 years ago

this filter lies

it says is change the max links allow but changes the number of links found

5 years ago

updated function

#2 @pbearne
5 years ago

  • Keywords has-patch added; needs-patch removed

OK we looked at this and the filters name doesn't make sense

So we have deprecated the current filter and added 2 more to place it

one that allows's you to change / set the max links allowed

and a renamed filter that allows you to filter the number of links found in the comment

Plus whitespace tidy / code format for the rest of the function ( if this too many changes ask and I will remove the extra changes

We used patch as a demo at contrib2core toronto meetup

#3 @rachelbaker
5 years ago

  • Keywords needs-refresh added

@pbearne Thanks for the patch. While I agree that comment_max_links_url isn't the BEST filter name, I don't see why the scope of this ticket can't be kept to just updating the parameter description.

Additional feedback on comment.php.13.patch :

  1. Core already has pre_option_{name} filters (example: pre_option_comment_max_links) which should suffice most use-cases of the comment_max_links_allowed filter you added.
  2. Your patch should only include code changes, not additional style or formatting refactoring. See: https://make.wordpress.org/core/handbook/contribute/code-refactoring/

5 years ago

comment and new filter that is usefull

5 years ago

Just comment

#4 @ocean90
5 years ago

  • Keywords good-first-bug needs-refresh removed

@pbearne For the future, please ignore tickets with the good-first-bug keyword. Such tickets are designed for new contributors. If you have worked with new contributors let them comment and upload the patch.

#5 @pbearne
5 years ago

@rachelbaker sorry about the code tidy got carried away :-)

I pushed up 2 patches one with just the comment change

But a relooking at this I felt that the filter wasn't much use as the sort of use that would make to do do a recount with a different regx (eg not include links to .gov sites). So I also have added a patch that adds a new filter that also passes the functions args as well

#6 @rachelbaker
5 years ago

  • Milestone changed from Future Release to 4.6

#7 @rachelbaker
5 years ago

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

In 38098:

Docs: Correct comment_max_links_url filter and $url param descriptions to communicate values are found links.

$num_links is the number of link matches found within the comment_content, and that is the value that can be modified with the comment_max_links_url filter.

Props pbearne.
Fixes #37319.

Note: See TracTickets for help on using tickets.