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:'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:
/** * - '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?
'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'), ),
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | drupal.library-api.patch | 1.41 KB | sun |
Comments
Comment #1
aaron commentedUnless we go with class objects and properties, this is probably the next best thing.
Comment #2
sunAlso 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).
Comment #3
MichaelCole commented@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
Comment #4
MichaelCole commentedComment #5
sunRe-attaching here.
Comment #7
robloachIt 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.
Comment #8
dries commentedCommitted to CVS HEAD. Thanks.