Make WordPress Core

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#33878 closed enhancement (fixed)

New function: `get_attachment_image_url()`

Reported by: sebastianpisula's profile sebastian.pisula Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: minor Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description (last modified by swissspidy)

The attached patch adds a new get_attachment_image_url() function to retrieve the URL for a given attachment size.

Attachments (4)

get_attachment_image_url (922 bytes) - added by sebastian.pisula 9 years ago.
33878.diff (2.0 KB) - added by swissspidy 9 years ago.
33878_33070.diff (4.8 KB) - added by dipesh.kakadiya 9 years ago.
wp_get_attachment_image_url #33878, the_post_thumbnail_url and get_the_post_thumbnail_url #33070 function added for get image url
33878_33070.1.diff (4.8 KB) - added by dipesh.kakadiya 9 years ago.
Patch Updated #33878 #33070

Download all attachments as: .zip

Change History (14)

#1 @swissspidy
9 years ago

  • Description modified (diff)
  • Summary changed from Get attachment url to New function: `get_attachment_image_url()`
  • Type changed from enhancement to feature request
  • Version 4.3 deleted

Note that there's already wp_get_attachment_url() and wp_get_attachment_thumb_url().

My suggestion would be to make those functions a wrapper for wp_get_attachment_image_src() and then add a second $size parameter to wp_get_attachment_url(). Example:

wp_get_attachment_url( $id, $size = 'full' ) { ... wp_get_attachment_image_src() … }
wp_get_attachment_thumb_url( $id ) { return wp_get_attachment_url( $id, 'thumbnail' ); }

#2 @swissspidy
9 years ago

  • Keywords needs-patch added

#3 @swissspidy
9 years ago

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

Nevermind, wp_get_attachment_url() is not only for images. wp_get_attachment_image_url() as suggested sounds good.

@sebastian.pisula Would you mind refreshing the patch with proper inline docs and perhaps unit tests?

@swissspidy
9 years ago

#4 @swissspidy
9 years ago

  • Keywords needs-refresh removed

Attached a new patch with a single test for wp_get_attachment_image_url. Probably need tests for all the other attachment functions though, which would take a while...

Version 0, edited 9 years ago by swissspidy (next)

@dipesh.kakadiya
9 years ago

wp_get_attachment_image_url #33878, the_post_thumbnail_url and get_the_post_thumbnail_url #33070 function added for get image url

#5 @swissspidy
9 years ago

  • Keywords needs-unit-tests removed
  • Milestone changed from Awaiting Review to 4.4
  • Severity changed from normal to minor
  • Type changed from feature request to enhancement

@dipesh.kakadiya
9 years ago

Patch Updated #33878 #33070

#6 @wonderboymusic
9 years ago

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

In 34372:

Media: Add a new function, wp_get_attachment_image_url(), which is a shortcut for wp_get_attachment_image_src() - same function signature, but returns just the URL based on $size.

Adds unit test.

Props dipesh.kakadiya, swissspidy, sebastian.pisula.
Fixes #33878.

#7 @azaozz
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm sorry but why do we need this function?

  • It has confusing name: what's the difference between image_src and image_url? As far as I know these two are equivalent. We already had wp_get_attachment_image_src(), why do we need wp_get_attachment_image_url()?
  • It doesn't do anything. The only difference between wp_get_attachment_image_src() and wp_get_attachment_image_url() is that the former returns the image URL as an array value. Do we really need two functions that take exactly the same arguments and return exactly the same values just in a slightly different format? Looks like bloat?

#8 follow-up: @Frank Klein
8 years ago

I see three main reasons for introducing the function:

  1. Developer happiness. I don't know how often I've read or written similar code to that in wp_get_attachment_image_url(). So instead of jumping through those hoops, just calling a function for getting the URL is nice.
  2. More clarity. You are calling wp_get_attachment_image_src(), and then take the first element out of the returned array, which is the URL. If you use the second and third element, you get sizes. The last argument contains a boolean. That is pretty weird, I personally find the way that the function returns data very inelegant. wp_get_attachment_image_url() clearly states what you get, making code more readable.
  3. Ease of use. It's pretty impossible to know what the array returned by wp_get_attachment_image_src() contains without reading the documentation. It's also not super easy to know that you should use this function to retrieve an attachment URL, because what exactly does "source" mean? The new function is a lot more straightforward.

#9 in reply to: ↑ 8 @SergeyBiryukov
8 years ago

Replying to Frank Klein:

You are calling wp_get_attachment_image_src(), and then take the first element out of the returned array, which is the URL. If you use the second and third element, you get sizes. The last argument contains a boolean. That is pretty weird, I personally find the way that the function returns data very inelegant.

Yup, I've seen people on support forums trying to use wp_get_attachment_image_src() as a URL without realizing that it returns an array.

#10 @azaozz
8 years ago

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

Yeah, I agree wp_get_attachment_image_src() is not intuitive as it returns an array, and wp_get_attachment_image_url() makes it easier for people that do not want to read any docs (they still may get confused by the "clashing" names). Just not sure if we should encourage that :)

In any case, this has been in core for a few weeks. I should have seen it earlier.

Last edited 8 years ago by azaozz (previous) (diff)
Note: See TracTickets for help on using tickets.