WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#36571 closed enhancement (fixed)

Add `$comment` parameter to `get_comment_author_email_link()`

Reported by: flixos90 Owned by: DrewAPicture
Milestone: 4.6 Priority: normal
Severity: normal Version: 2.7
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

To use this function (also its related output function) for any specific comment, let's add a $comment parameter.

Attachments (5)

36571.diff (2.7 KB) - added by flixos90 4 years ago.
patch adding the parameter
36571.2.diff (3.0 KB) - added by flixos90 4 years ago.
improved docs
36571.3.diff (2.4 KB) - added by DrewAPicture 4 years ago.
Docs
36571.4.diff (5.5 KB) - added by DrewAPicture 4 years ago.
Tests
36571.5.diff (5.8 KB) - added by boonebgorges 4 years ago.

Download all attachments as: .zip

Change History (17)

@flixos90
4 years ago

patch adding the parameter

#1 @flixos90
4 years ago

  • Keywords has-patch added

@flixos90
4 years ago

improved docs

#2 follow-up: @SergeyBiryukov
4 years ago

What about comment_author_url_link() and get_comment_author_url_link()?

#3 @boonebgorges
4 years ago

I know that the default value of 0 for $comment is intended to trigger the empty() check at the beginning of get_comment(). IMHO it would be more transparent if the default value were null, since this would indicate that no value is being provided (rather than a value of 0, which makes me do a double-take when I read the function signature, since a comment can't have an ID of 0).

#4 in reply to: ↑ 2 @flixos90
4 years ago

Replying to SergeyBiryukov:

What about comment_author_url_link() and get_comment_author_url_link()?

I created a separate ticket for that in #36573.

#5 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 4.6

#6 @wonderboymusic
4 years ago

  • Keywords needs-unit-tests added

#needs-unit-tests - there currently are none

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


4 years ago

#8 @DrewAPicture
4 years ago

36571.3.diff removes the spacing changes to other paramter docs – makes for a cleaner, easier to grok commit. We'll follow up and fix the spacing. Also, $comment in both instances is a new parameter in core nomenclature. Arguments are sub-parameter in this context :-)

@DrewAPicture
4 years ago

Docs

#9 @DrewAPicture
4 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

36571.4.diff adds tests.

@DrewAPicture
4 years ago

Tests

@boonebgorges
4 years ago

#10 @boonebgorges
4 years ago

36571.5.diff makes the following changes to 36571.4.diff :

  • Share the same comment fixture across the new tests (using wpSetUpBeforeClass() etc)
  • During test teardown, clear the $comment global and readd the antispambot filter, to avoid test pollution
  • Use null for a default value of the new param rather than 0
  • Only use the @ticket flag for test methods that are directly related to the enhancement being introduced here

Changes are looking good to me.

#11 @DrewAPicture
4 years ago

In 37348:

Comments: Adjust comment_author_email_link() and get_comment_author_email_link() to each accept a new optional fourth parameter, $comment, which enables overriding the $comment global.

Adds tests.

Props flixos90, boonebgorges, DrewAPicture.
See #36571.

#12 @DrewAPicture
4 years ago

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

In 37349:

Comments: Realign parameter documentation in the DocBlocks for comment_author_email_link() and get_comment_author_email_link() following [37348].

Also adds a missing return description for get_comment_author_email_link().

Props flixos90 for the initial patch.
Fixes #36571.

Note: See TracTickets for help on using tickets.