Opened 4 years ago
Closed 4 years ago
#52603 closed defect (bug) (fixed)
Inconsistent doc for return value of wp_generate_attachment_metadata
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | 2.5 |
Component: | Media | Keywords: | has-patch commit |
Focuses: | docs | Cc: |
Description
The return value of wp_generate_attachment_metadata()
is documented as mixed. However, the filter wp_generate_attachment_metadata
documents it as array. So I would expect that the return value would be documented as array too.
Attachments (3)
Change History (15)
#2
@
4 years ago
- Milestone changed from Awaiting Review to 5.8
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
#4
@
4 years ago
This is a documentation only change, and can be made during the beta part of the release cycle, so going to leave this one in the milestone, even though today is beta 1.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by desrosj. View the logs.
4 years ago
#10
follow-up:
↓ 11
@
4 years ago
- Keywords commit added; needs-refresh removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for a small coding standards follow-up to [51162].
Patch 52603.3.diff reformats the multi-line comment to use /*
instead of //
.
Marking the patch for commit cc @SergeyBiryukov
#11
in reply to:
↑ 10
@
4 years ago
Replying to hellofromTonya:
Reopening for a small coding standards follow-up to [51162].
Patch 52603.3.diff reformats the multi-line comment to use
/*
instead of//
.
Thanks for the patch!
As previously noted in comment:1:ticket:53004 and comment:9:ticket:52025, there are many other instances in core of the single-line comment style being used for multi-line comments.
There is a section in the documentation standards that clarifies which format is generally recommended, but that is just a recommendation and not a strict rule.
This was previously corrected for most of inline comments in core in [47122]. I think I opted to keep comments with one or two lines in single line format, and switch comments with three or more lines to the multi-line format.
At this time, until we have a WPCS rule that forbids using this comment style for multi-line comments, I think it makes sense to keep using single line format for comments with one or two lines.
That said, I have no objections converting this comment to three lines :)
Thanks for the patch!
It looks like
wp_generate_attachment_metadata()
can actually returnfalse
for an audio or video attachment that doesn't exist in the local filesystem, as previously noted in comment:5:ticket:25649.Some history here:
Since that was never documented neither in the function nor in the
wp_generate_attachment_metadata
filter, I think we have two options here:wp_generate_attachment_metadata
filter says.false
for an audio or video attachment that doesn't exist in the local filesystem.For consistency with other functions like
wp_create_image_subsizes()
, I think the first option would be preferable here, see 52603.2.diff. Looking at thewp_generate_attachment_metadata()
instances in core, we don't really expect it to return anything other than an array.P.S. To complicate things a bit further,
wp_read_image_metadata()
does returnfalse
on failure, same aswp_read_audio_metadata()
andwp_read_video_metadata()
, but that's probably not super relevant here.