Currently postponed on #958162: Allow groups of callbacks to be applied to libraries.

Hence, also no patch, because it doesn't make sense to roll one yet.

The main piece of a patch will be the following function, though. The rest should be more or less trivial:

 /**
 * Helper callback to make 'files' property of libraries consistent.
 *
 * This turns libraries' file information declared as e.g.
 * @code
 * $library['files']['js'] = array('example_1.js', 'example_2.js');
 * @endcode
 * into
 * @code
 * $library['files']['js'] = array(
 *   'example_1.js' => array(),
 *   'example_2.js' => array(),
 * );
 * @endcode
 * It does the same for the 'integration files' property.
 *
 * @param $library
 *   An associative array of library information or a part of it, passed by
 *   reference.
 * @param $version
 *   If the library information belongs to a specific version, the version
 *   string. NULL otherwise.
 * @param $variant
 *   If the library information belongs to a specific variant, the variant name.
 *   NULL otherwise.
 *
 * @see libraries_info()
 * @see libraries_invoke()
 */
function libraries_prepare_files(&$library, $version, $variant) {
  // Both the 'files' property and the 'integration files' property contain file
  // declarations, and we want to make both consistent.
  $file_types = array();
  if (isset($library['files'])) {
    $file_types[] = &$library['files'];
  }
  if (isset($library['integration files'])) {
    // Integration files are additionally keyed by module.
    foreach ($library['integration files'] as &$integration_files) {
      $file_types[] = &$integration_files;
    }
  }
  foreach ($file_types as &$files) {
    // Go through all supported types of files.
    foreach (array('js', 'css', 'php') as $type) {
      if (!empty($files[$type])) {
        foreach ($files[$type] as $key => $value) {
          // Unset numeric keys and turn the respective values into keys.
          if (is_numeric($key)) {
            $files[$type][$value] = array();
            unset($files[$type][$key]);
          }
        }
      }
    }
  }
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Postponed » Needs review
FileSize
5.91 KB

Reroll, now that there's some of #958162: Allow groups of callbacks to be applied to libraries in master.
I don't know if we want to test the callback specifically, because it is already tested indirectly as can be seen from the hunks in libraries.test.

tstoeckler’s picture

Status: Needs review » Needs work
+++ b/libraries.module
@@ -204,6 +204,64 @@ function libraries_traverse_library(&$library, $callback) {
+      if (!empty($files[$type])) {

Not that it matters, but I think a isset() would do here.

tstoeckler’s picture

Status: Needs work » Needs review
FileSize
5.64 KB

Here's a new reroll after the commit in the callbacks issue. This one comes with a dedicated test, which can't hurt. Two things:
1. As can be seen from the first libraries.test hunk, libraries_prepare_files() (and any other callback) can't be called like: libraries_prepare_files($library) because in the function declaration we don't set defaults for the $version and $variant parameter that get passed in the callback invoking. We could of course do that, but then we should probably document that in libraries.api.php, something like:

An example callback function declaration might look like:
function mymodule_callback($library, $version = NULL, $variant = NULL)

Not that that would be inherently bad, I'm just not sure it's necessary, as these functions are (almost ?) never called except by libraries_invoke (and in tests). Maybe I'm also making an elephant out of this, and we can assume people who write modules know what argument defaults are...

2. We should really split the first part of the tests into a LibrariesUnitTestCase extends DrupalUnitTestCase, but probably not in this issue.

Status: Needs review » Needs work

The last submitted patch, 1023258-3-prepare-files(1).patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
FileSize
5.57 KB

Now with less whitespace.
(I just figured out the nano auto-indent feature :( ....)

tstoeckler’s picture

Regarding my #4.1:
I think we should add those defaults in the function header and document them, then we could remove the weird-looking $callback($library, NULL, NULL); from libraries_traverse_library(). Eager to hear your thoughts, though. (And of course, it's also extreeeemely (!) minor, so we might as well commit this and discuss it in detail in about 1.5 years, when we've got all other more important issues sorted out ... :))

tstoeckler’s picture

Status: Needs review » Fixed
FileSize
7.06 KB

I stumbled over this again and wanted to get it over with :), so I committed the above patch.
I did a small modification in that I added documentation to specify default values for $version and $variant. I left libraries_traverse_library() explicitly calling the functions with NULL parameters for now, if we want to we can remove that in a follow-up. Patch with the small modification attached.

tstoeckler’s picture

Oh, I just noticed I messed up the commit:
http://drupalcode.org/project/libraries.git/commit/49d97ea

Sorry for that...

sun’s picture

Committed attached follow-up patch.

tstoeckler’s picture

Oops, that's weird :)...
Thanks!

Status: Fixed » Closed (fixed)

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