WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 7 months ago

#25275 new enhancement

Checking if attachment is audio/video

Reported by: danielpataki Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.6
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Hi Everyone!

This is my first time attaching a diff, if you have a moment, please let me know if I am doing it wrong,

I am building a theme just now which needs to check if an attachment is audio. It seems that only image format functions are included in post.php (wp_attachment_is_image()) so I quickly created one for audio and video.

I am not a huge expert on formats, so if you have any more to add, please let me know!

Attachments (1)

post.diff (1.5 KB) - added by danielpataki 7 months ago.
Final version of diff to add audio and video checking to attachments in post.php

Download all attachments as: .zip

Change History (10)

comment:1 rmccue7 months ago

Thanks for the patch!

Two small issues with it:

  1. $image_exts is supposed to be called that; it stands for image extensions, not image exists
  2. You appear to have accidentally duplicated the function name, I believe they should be wp_attachment_is_audio and wp_attachment_is_video instead.

(Apparently the filename has also disappeared from the diff, not quite sure why for that though.)

Last edited 7 months ago by rmccue (previous) (diff)

comment:2 danielpataki7 months ago

Sorry about that, I only made 10 errors in 20 lines of code, I must be nervous to add a diff :D I had a detailed look at the changes, all should be well now.

Is there anything I should do differently when contributing to make peoples' lives easier? (apart from not leaving errors in there)?

Thanks :)

danielpataki7 months ago

Final version of diff to add audio and video checking to attachments in post.php

comment:3 SergeyBiryukov7 months ago

  • Component changed from General to Media
  • Keywords has-patch added; attachments removed
  • Summary changed from Checking if attachment is adio/video to Checking if attachment is audio/video

When uploading a new version of a patch, please don't overwrite the old one, so that context is preserved.

comment:4 danielpataki7 months ago

Hi Sergey,

I normally would not have done that, the only reason I overwrote was because the patch had errors like an extra comma after an array and so on. I will definitely not overwrite at all from now on!

Daniel

comment:5 DrewAPicture7 months ago

Hi Daniel,

Thanks for the patch, just a couple of suggestions:

  • Since it looks like *_video() and *_audio() are essentially verbatim copies of wp_attachment_is_image() with some small changes, it might make more sense to deprecate *_image() and combine all three into a single function with a type flag, maybe something like wp_attachment_is_type().
  • Also, we could probably just use wp_ext2type() to determine the extension type because it opens the door for future types to be added to a *_type() function. It looks like you forgot to change image/ to video/ or audio/ in the type checks anyway.
  • Last, in the future, please generate patches from the WordPress root :)

comment:6 DrewAPicture7 months ago

  • Cc xoodrew@… added

comment:7 danielpataki7 months ago

Agreed on all counts! I will modify the code to reflect al that.

I'll add a wp_attachment_is_type() function which uses wp_ext2type() internally.

Sorry for being a bit of a dunce but I'm completely new to making patches. When you say "generate patches from the WordPress root", does this mean to diff the whole directory? I basically used: diff -u post.php post-mine.php > post.diff to create the diff file. Would the method to generate from the root be to have a vanilla copy of wordpress and another one where I make changes side by side and then just use this: diff -u wordpress wordpress-mine > wordpress-diff.

Also, since wp_ext2type is only in alpha, do you recommend always creating patches from an SVN checkout of the trunk?

Sorry for all the questions!

Daniel

comment:8 nacin7 months ago

$post->post_mime_type?

comment:9 danielpataki7 months ago

Hi nacin,

I did think about using the mime type but I think a more general-purpose function might be a more reliable and more flexible solution. The mime type would work pretty well for audio and video, but wouldn't be too useful for documents, archive files and so on. I addition some files like markdown files don't have an official mime type.

By using the filter in wp_ext2type() users could customize the thing completely.

Let me know if I am completely unaware of how mime types work, I am not very familiar with them :(

Daniel

Note: See TracTickets for help on using tickets.