hook_library_info_alter() docs and function signature are slightly out of date

1. +++ b/core/modules/system/theme.api.php
   @@ -848,10 +848,10 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + *   The name of the extension that registered a library.
   

   Here it says the library.

2. +++ b/core/modules/system/theme.api.php
   @@ -848,10 +848,10 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   +function hook_library_info_alter(&$libraries, $extension) {
   

   While here it says the libraries.

@joshi.rohit100: I think you did an interdiff of the new patch vs the old patch, the lines you added are marked red. Thanks for fixing this so quickly.

I'll let Cottser review the patch as well. Looks good to me now.

Makes sense to me, but passing to Jennifer just in case I'm missing something.

This does make sense to me too, but maybe we should document that it can also be 'core', as in the example code? Because 'core' is not really an extension, per se.

Also to be very very clear, maybe it should say the "machine name"?

By the way, I verified from looking at https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Asset!LibraryDisc... and functions it calls that indeed, the extension name passed into this hook is either 'core' or a module or theme machine name. So this change is good, maybe just not quite complete.

Hook should mention about modules and themes because both able to use this hook.
Also core use notion of asset for libraries so I changed a description.
The related protected method that calls that hook should be referenced.
No interdiff because patch totally different

PS: please check my grammar ;(

Thanks! This is looking better, and the grammar is pretty close. ;)

Still a couple of issues to address in the documentation:

1. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,25 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * Alters assets (JavaScript/CSS) registry.
   

   Why are we calling this the "assets registry"? The terminology in the rest of the code, function names, etc. is "library" so let's keep it at that -- don't introduce new terms. So this should be:

   Alter JavaScript/CSS library information.

   [note: verb should be "Alter" not "Alters"]

2. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,25 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * update library to newer version while ensuring backwards compatibility. In
   

   Several grammar/punctuation/typo fixes needed in this line. Should be:

   update a library to a newer version, while ensuring backward compatibility.

3. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,25 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * general, such manipulations should only be done by designated modules and
   + * themes, since most modules and themes that integrate with a certain library
   + * also depend on the API of a certain library version.
   + *
   

   What is a "designated" module or theme? I don't understand what this is trying to say, exactly.

4. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,25 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + *   The name of the extension that registered the libraries.
   

   For clarity, should say "machine name" not just "name" here.

   Also, since "core" is not an extension, that should be mentioned in this documentation.

Attached wrong patch in #14.

When you upload a patch on an issue that already had a patch, please make an Interdiff file. Thanks!

Anyway... thanks for the new patch, which is an improvement. But it still needs some work:

1. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * update a library to a newer version, while ensuring backward compatibility.In
   

   There needs to be a space after the . here -- and then the line will be over 80 characters so you will need to rewrap the paragraph.

2. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * update a library to a newer version, while ensuring backward compatibility.In
   + * general, such manipulations should only be done by the modules and themes
   + * that will extend this functionality, since most modules and themes that
   + * integrate with a certain library also depend on the API of a certain library
   + * version.
   + *
   

   Hm. The wording here is a bit confusing, but at least I see what the intent is this time. ;)

   How about something like this:

   In general, such manipulations should only be done to extend the library's functionality in a backward-compatible way, to avoid breaking other modules and themes that may be using the library.

3. +++ b/core/modules/system/theme.api.php
   @@ -838,22 +838,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * @param string $extension
   + *   The machine name of the extension that registered the libraries.
   

   Again, please say something about core here, which is not an extension. See several of my previous reviews, which made the same comment, and which still hasn't been addressed... and just to point out, that was the whole point of this issue actually!

The related issue adds another way to override libraries so probably needs to be mentioned here somehow

I'd like to point that render api separates assets and attachments so better to get feedback from @Wim Leers at least

+++ b/core/modules/system/theme.api.php
@@ -838,22 +838,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
+ * Alter JavaScript/CSS library information.

That should point that alter happens on extension's libraries (multiple)

That other issue mentioned in #18 has not been finalized. It should be updating the related documentation when it is finalized, right? We can't document something here that isn't yet part of Core.

Made suggested changes mentioned in #17 to most recent patch.

+++ b/core/modules/system/theme.api.php
@@ -847,22 +847,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
+ * Alter JavaScript/CSS library information.

and again, Alter libraries provided by extension....

Right, I missed that. How's this then?
- Made change #23 suggested.

Looks good, thanks!

Two very minor grammar/punctuation/typo things to fix:

1. +++ b/core/modules/system/theme.api.php
   @@ -847,22 +847,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * Alter libraries provided by extension.
   

   Should be "an extension" or "extensions".

2. +++ b/core/modules/system/theme.api.php
   @@ -847,22 +847,26 @@ function hook_js_settings_alter(array &$settings, \Drupal\Core\Asset\AttachedAss
   + * Allows modules and themes to change libraries' definitions, mostly used to
   + * update a library to a newer version, while ensuring backward compatibility.
   + * In general, such manipulations should only be done to extend the library's
   

   the comma in this first line should be a ;

Made changes as per #25.

+1 rtbc

Added Beta Evaluation

+1 for RTBC.

No, actually I'll just commit this. :) Thanks everyone!