Problem/Motivation

The API documentation for hook_file_mimetype_mapping_alter() still refers to two functions which were removed by #1921558: Convert file_get_mimetype() to use Symfony MimeTypeGuessers.

  • file_mimetype_mapping()
  • file_default_mimetype_mapping()

Proposed resolution

Update the docblock for hook_file_mimetype_mapping_alter() to refer to the relevent parts of the new ExtensionMimeTypeGuesser class.

Comments

andrewmacpherson’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new1.2 KB
jhodgdon’s picture

Status: Needs review » Needs work

Thanks!

Our documentation standards currently require that when you refer to a class in documentation blocks, you include the full namespace of the class (starting with \ ), even in the text.

Other than that, it looks fine...

andrewmacpherson’s picture

Status: Needs work » Needs review
StatusFileSize
new2.44 KB

This patch updates the class names to the fully qualified format per #2, plus some slight rewording to make it fit the 80-character limit better.

It turns out there's another reference to file_mimetype_mapping(), but in he docblock for a function which is itself slated for removel. I've updated the docblock for the deprecated file_get_mimetype(), so it no longer directly refers to file_mimetype_mapping().

amitgoyal’s picture

Minor fixes to remove extra space and better utilization of available space.

jhodgdon’s picture

Looks good to me now, thanks! ... Well, we normally say hooks are "invoked" rather than "called". Want to clean that up?

+++ b/core/modules/system/system.api.php
@@ -2303,16 +2303,17 @@ function hook_install_tasks_alter(&$tasks, $install_state) {
 /**
  * Alter MIME type mappings used to determine MIME type from a file extension.
  *
- * This hook is run when file_mimetype_mapping() is called. It is used to
- * allow modules to add to or modify the default mapping from
- * file_default_mimetype_mapping().
+ * Called by \Drupal\Core\File\MimeType\ExtensionMimeTypeGuesser::guess(). It is
jhodgdon’s picture

I'd also like to ask that this patch not be committed until #2299715: [meta] Move core hooks from system.api.php to core.api.php or other files is complete. That's a big patch and it's moving this hook.

amitgoyal’s picture

StatusFileSize
new2.67 KB
new680 bytes

Sure. Fixes as per #5.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks better, thanks! Again, I'd like to ask for this not to be committed until #2299715: [meta] Move core hooks from system.api.php to core.api.php or other files is dealt with, at which point this will need a reroll.

webchick’s picture

Status: Reviewed & tested by the community » Postponed

Ditto.

jhodgdon’s picture

Status: Postponed » Reviewed & tested by the community

Never mind, the other patch already doesn't apply.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed d29adc8 and pushed to 8.x. Thanks!

  • alexpott committed d29adc8 on 8.x
    Issue #2297947 by amitgoyal, andrewmacpherson: Clean up API docs for...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.