Make WordPress Core

Opened 14 years ago

Closed 11 years ago

Last modified 11 years ago

#23882 closed enhancement (fixed)

get_permalink() accepts an ID, the_permalink() does not

Reported by: johnjamesjacoby Owned by: chriscct7
Priority: normal Milestone: 4.4
Component: Permalinks Version: 3.5
Severity: normal Keywords: has-patch commit dev-feedback
Cc: Focuses:

Description (last modified by johnjamesjacoby)

I always found it odd that the_permalink() doesn't accept a post ID passed into it, when get_permalink() does, and the_permalink() just calls get_permalink().

After scouring trac for a duplicate (and not finding one) I've attached a patch to improve this.

Related: #1271

Attachments (3)

23882.patch​ (611 bytes ) - added by johnjamesjacoby 14 years ago.
Add $id and $leavename params to the_permalink(), to match get_the_permalink()
23882.2.patch​ (773 bytes ) - added by chriscct7 11 years ago.
23882.3.patch​ (864 bytes ) - added by chriscct7 11 years ago.
Added $id param to the filter and set $id's default to false to match get_permalink

Download all attachments as: .zip

Change History (18)

@johnjamesjacoby
14 years ago

Add $id and $leavename params to the_permalink(), to match get_the_permalink()

#1 @SergeyBiryukov
14 years ago

  • Version trunk → 3.5

#2 @nacin
14 years ago

get_the_permalink() is actually get_permalink(). I don't think it's necessary for $leavename — which is designed for creating sample and generic permalinks, etc — to make its way into the_permalink(). Fine with $id.

#3 @jb510
14 years ago

Thank you... drove me crazy for a long time too -- patch worked in my basic testing.

#4 @johnjamesjacoby
13 years ago

  • Description modified (diff)
  • Summary get_the_permalink() accepts an ID, the_permalink() does not → get_permalink() accepts an ID, the_permalink() does not

#5 @chriscct7
11 years ago

  • Owner set to chriscct7
  • Status new → assigned

#6 @chriscct7
11 years ago

  • Keywords commit added
  • Milestone Awaiting Review → 4.4

Refreshed with just the single $id param per comment:2

@chriscct7
11 years ago

#7 @SergeyBiryukov
11 years ago

  • Resolution → fixed
  • Status assigned → closed

In 34982:

Add $id parameter to the_permalink(), for consistency with get_permalink().

Props johnjamesjacoby, chriscct7.
Fixes #23882.

#8 follow-up: @anonymized_8769252
11 years ago

  • Keywords dev-feedback added
  • Resolution fixed
  • Status closed → reopened

I'm not sure about this change, and I think it should be reverted, and the ticket closed as won't fix.

the_permalink() is a Template Tag, and as such always needs to be used in the Loop. It relies on the current global state being set to output the correct link. Having an ID parameter would change this, and make the function signature different from the other template tags.

Also currently the function has a the_permalink filter. Adding the ID parameter would change the links that are filtered, as this might be links to the current post or another post. I'm not sure this won't break existing code relying on this being the case. Also the filter is not useful without the parameters passed to the function.

Finally if you are in a template and want to output content that is different from the one in the loop, you can use the getter functions. That's a pretty established pattern in WordPress because a lot of template tags, at least the more recent, exclusively rely on the Loop, and this change breaks this common pattern.

#9 @chriscct7
11 years ago

the_permalink doesn't do anything that get_permalink doesn't already do by default. In fact, as it stands, it's simply a wrapper for get_permalink:

	function the_permalink() {
	        /**
	         * Filter the display of the permalink for the current post.
	         *
	         * @since 1.5.0
	         *
	         * @param string $permalink The permalink for the current post.
	         */
	        echo esc_url( apply_filters( 'the_permalink', get_permalink() ) );
	}

The patch doesn't do anything to change the behavior of the parameter. This is because if no parameter is passed in for ID, the 0 is passed into get_permalink which is the default value get_permalink gets in anyways in the event of no $id value, thus there is no backwards compatibility concerns on it.

In order to have any sort of changed functionality, you'd have to send in a value for the parameter of the_permalink, in which case, which is new functionality, and is being explicitly set, so the the_permalink filter isn't a concern here. I'll update the patch to add the parameter.

@chriscct7
11 years ago

Added $id param to the filter and set $id's default to false to match get_permalink

#10 in reply to: ↑ 8 @chriscct7
11 years ago

Replying to Frank Klein:

I'm not sure about this change, and I think it should be reverted, and the ticket closed as won't fix.

the_permalink() is a Template Tag, and as such always needs to be used in the Loop. It relies on the current global state being set to output the correct link. Having an ID parameter would change this, and make the function signature different from the other template tags.

the_permalink is simply a wrapper for get_permalink()

Also currently the function has a the_permalink filter. Adding the ID parameter would change the links that are filtered, as this might be links to the current post or another post. I'm not sure this won't break existing code relying on this being the case. Also the filter is not useful without the parameters passed to the function.

The ID parameter only changes the functionality of the function if you use it, which would be new, and thus non breaking. Using the ID parameter in get_permalink is the same way. Added a patch to add the parameter

There's nothing in this patch that changes any existing behavior, it simply adds new functionality (similar to adding additional arguments to the WP_Query).

#11 @chriscct7
11 years ago

It should be noted there's alot of precedent for supporting an $id parameter. Many of the template functions do as well, for example the_title_attribute. Further, the use of $id is something that many users as a wrapper function expect it to support. There's an early trac ticket on this, and now that I notice it, several Codex articles that assume it supports $id already, like the code example on ​https://codex.wordpress.org/Template_Tags/the_category_rss.

#12 follow-up: @anonymized_8769252
11 years ago

I am aware that the_permalink() is simply a wrapper for the getter function. The Template Tags are WordPress' version of a templating engine, so a lot of the functions just wrap the getters and rely on the Loop to print the correct data.

the_title_attribute() is a bad example IMO, because it has an echo parameter, so it can be used both as a getter and a Template Tag.

I also feel that incorrect code samples in documentation is not really a valid argument for extending an API.

I'm also aware that besides the filter argument, which IMO definitely does modify existing behavior, this function does not change if you don't use the ID parameter.

But I think that adding a parameter would change the function from being a pure Template Tag towards something that is mixed. This would break with the existing conventions of how templating works (for the most part due to backwards compatibility reasons) in WordPress, and therefore isn't a worthwhile addition because the gain of adding the parameter in view of these downsides isn't worth it.

#13 in reply to: ↑ 12 @DrewAPicture
11 years ago

  • Resolution → fixed
  • Status reopened → closed

Replying to Frank Klein:

But I think that adding a parameter would change the function from being a pure Template Tag towards something that is mixed. This would break with the existing conventions of how templating works (for the most part due to backwards compatibility reasons) in WordPress, and therefore isn't a worthwhile addition because the gain of adding the parameter in view of these downsides isn't worth it.

I recognize your concerns about the purity of what it means to have template tags, at the same time, I don't see a downside or a slippery slope here.

This change makes the_permalink() more flexible without affecting default behavior. It also allows people already using it in the loop to continue using what they know in other contexts. If we can save people time having to hunt down a non-template tag for the sake of semantics, that's a win, in my opinion.

Reclosing as fixed.

#14 @DrewAPicture
11 years ago

#34234 was opened to discuss renaming $id in the_permalink(), get_the_permalink(), and get_permalink() to $post. The ticket will also address @chriscct7's suggested 23882.3.patch​ for adding the parameter to the filter.

#15 @DrewAPicture
11 years ago

In 35002:

Template: Pass the $post parameter to the the_permalink filter.

Props chriscct7.
Fixes #34234. See #23882.

Note: See TracTickets for help on using tickets.