Download & Extend

Incorrect documentation for hook_library()

Project:Drupal core
Version:7.x-dev
Component:javascript
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Within #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries), jpetso brought up an issue that it's difficult to alter an array list for the dependencies. Should we switch to an associative list here instead?

Not sure about the format of "dependencies", see this piece of code that was committed to system.api.php:

<?php
'dependencies' => array(
 
// Require jQuery UI core by System module.
 
array('system' => 'ui'),
 
// Require our other library.
 
array('my_module', 'library-1'),
 
// Require another library.
 
array('other_module', 'library-3'),
),
?>

Note the associative arrow "=>" in the "system" example and the "," in the other ones. The documentation also hasn't got the clearest of all wordings on this matter:

<?php
/**
* - 'dependencies': An array of libraries that are required for a library. Each
*   element is an array containing the module and name of the registered
*   library. Note that all dependencies for each dependent library will be
*   added when this library is added.
*/
?>

I think this error in the example might highlight a source of potential errors, other developers could easily make the same mistake. Maybe a format like the following one would be more appropriate?

<?php
'dependencies' => array(
 
// Require jQuery UI core by System module.
 
'system' => array('ui'),
 
// Require our other library.
 
'my_module' => array('library-1'),
 
// Require another library.
 
'other_module' => array('library-3'),
),
?>

Comments

#1

Unless we go with class objects and properties, this is probably the next best thing.

#2

Category:feature request» task
Status:active» needs review

Also looks like my patch from #315100-285: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) was not committed (which at least fixed the docs).

#3

@sun, the #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) was committed in it's #281.

In it's #291, Rob Loach says "Thanks for giving that a look, Gabor.... http://drupal.org/update/modules/6/7#drupal_add_library . I also updated http://drupal.org/update/modules/6/7#jquery_ui ."

Is there anything left to work on for this bug?

I don't see anything to review...

Mike

#4

Status:needs review» needs work

#5

Status:needs work» needs review

Re-attaching here.

AttachmentSizeStatusTest resultOperations
drupal.library-api.patch1.41 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details

#6

Status:needs review» needs work

The last submitted patch, drupal.library-api.patch, failed testing.

#7

Title:Associative Array for the Libraries?» Incorrect documentation for hook_library()
Status:needs work» reviewed & tested by the community

It would be nice if this was an associative array, but unfortunately it's not. I think we should just fix the documentation here, and possibly open up a new issue if we want to change the API later on. I'm setting this to RTBC.

#8

Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks.

#9

Status:fixed» closed (fixed)

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