Inconsistent doc for return value of wp_generate_attachment_metadata

[Chouby]

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.

[SergeyBiryukov]

Thanks for the patch!

It looks like wp_generate_attachment_metadata() can actually return false 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:

• [23766] / #23673
• [25968] / #25649
• [35554] / #34599

Since that was never documented neither in the function nor in the wp_generate_attachment_metadata filter, I think we have two options here:

• Make sure the function indeed always returns an array, as the wp_generate_attachment_metadata filter says.
• Document that it returns 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 the wp_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 return false on failure, same as wp_read_audio_metadata() and wp_read_video_metadata(), but that's probably not super relevant here.

[desrosj]

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.

[SergeyBiryukov]

In 51162:

Media: Make sure wp_generate_attachment_metadata() always returns an array.

This matches the documentation for the filter of the same name.

Previously, the function could return false for an audio or video attachment that does not exist in the local filesystem.

Props Chouby, SergeyBiryukov.
Fixes #52603.

[hellofromTonya]

Reformats multi-line comment to use /* instead of per coding standards.

[hellofromTonya]

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

[SergeyBiryukov]

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 :)

[SergeyBiryukov]

In 51166:

Docs: Update syntax for multi-line comment in wp_generate_attachment_metadata() per the documentation standards.

Follow-up to [23766], [25968], [35554], [51162].

Props hellofromTonya.
Fixes #52603.