add wp_attachment_is_audio() and wp_attachment_is_video()

[pbiron]

Core already has ​wp_attachment_is_image().

For consistency and convenience, core should also have wp_attachment_is_audio() and wp_attachment_is_video().

[pbiron]

In addition to adding wp_attachment_is_audio() and wp_attachment_is_video(), the patch also replaces all uses of wp_attachment_is( 'audio' ) and wp_attachment_is( 'video' ) with the new functions (except in unit tests, where the new functions are added to the relevant existing tests that use wp_attachment_is()).

[kirasong]

I personally prefer the generic nature of wp_attachment_is() (it looks like this was an intentional move) to having a separate function each of many eventual file types.

Is there a particular reason it'd be helpful to have functions for these, specifically?

[pbiron]

Replying to mikeschroder:

I personally prefer the generic nature of wp_attachment_is() (it looks like this was an intentional move) to having a separate function each of many eventual file types.

Yes, I think the introduction of wp_attachment_is() was intentional in 4.2.0...and wp_attachment_is_image() was changed to be a simple wrapper around that in 4.2.0.

Is there a particular reason it'd be helpful to have functions for these, specifically?

As I said, the reason is consistency...why have wp_attachment_is_image() but not these others?

I'd be fine with deprecating wp_attachment_is_image(), but if it's not going to be deprecated then I think the others should be added.

[pbiron]

Replying to pbiron:

Is there a particular reason it'd be helpful to have functions for these, specifically?

As I said, the reason is consistency...why have wp_attachment_is_image() but not these others?

I'd be fine with deprecating wp_attachment_is_image(), but if it's not going to be deprecated then I think the others should be added.

There are currently 8 uses of wp_attachment_is_image() in core...so if it does get deprecated those uses would need to be changed to wp_attachment_is( 'image' ).

[SergeyBiryukov]

Re-reading #25275, deprecating wp_attachment_is_image() was indeed suggested, but I don't think switching to a more generic function is a strong enough reason to do that.

wp_attachment_is() was introduced as an alternative to duplicating the same code in three different functions.

Since duplication is no longer a concern here, I don't have any objections to adding these wrappers (maybe without replacing the existing wp_attachment_is() usage though).

That said, introducing them means that any new formats added to wp_attachment_is() in the future (e.g. pdf) would probably need a corresponding wp_attachment_is_*() function for consistency.

[joemcgill]

After some discussion here, as well as the previous discussion in #25275 and in ​Slack, the general consensus seems to be that there isn't currently a compelling reason to add these functions at this time. The desire to create consistency across the codebase with wp_attachment_is_image() is reasonable, but the burden required to maintain this consistency by adding new functions for each supported attachment type over time, along with the very likely potential of these wrapping functions diverging over time, is more persuasive at this time. However, if our thinking shifts in the future, 41802.diff​ is a model for how we would want to execute this.

Closing for now as maybelater.

Any related discussion about deprecating wp_attachment_is_image() should happen in a separate ticket.