Correct documentation of ThemeNegotiatorInterface

Could this be a first approach?

Thanks for the patch and the review comment!

So since this was written, it looks like someone actually made an interface for this, which is what should have been there all along.

In which case, instead of saying essentially, in this paragraph, "These are the methods you need to implement and here is what they need to do", what we should do is just say that the service class needs to implement \Drupal\Core\Theme\ThemeNegotiatorInterface -- which gives exactly that information.

And while we are fixing this, can we instead of saying "see user.services.yml for an example", be more explicit and give the service name and the class that implements it? Thanks!

Apply changes mentioned on #2.

Apply changes mentioned on #4.
Here I had some doubts, I did not know how to document this in a more correct way.

1. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -12,10 +12,9 @@
   + * (this service uses the class \Drupal\user\Theme\AdminNegotiator). The service
   

   Um, sorry I guess I wasn't clear.

   The part in parentheses needs to say something like:

   (see the xyz.whatever.whatever service in user.services.yml for an example).

2. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -12,10 +12,9 @@
   + * class needs to implement \Drupal\Core\Theme\ThemeNegotiatorInterface.
   

   Doh. I just realized that this documentation is on the interface itself.

   So this probably just needs to say somewhere "implementing this interface" rather than having a link to itself!

3. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -46,7 +45,8 @@ public function applies(RouteMatchInterface $route_match);
   +   *   Return the name of the theme, or NULL if other negotiators like the
   +   *   configured default one should kick in instead.
   

   needs comma before "like" and another one after "one".

   Actually we don't normally start @return docs with "Return...". Please fix.

Thanks @jhodgdon and my apologies for the mistakes.
This patch fix 1 and 3 on #6.

The second point maybe could be something like this:
"Implementing this interface the only methods this service needs to implement are applies and determineActiveTheme."

(The second point has not been resolved yet. Change this point to leave it as it was on #2.)

Um. If you implement an interface, it means you need to implement all of the methods on the interface. I don't think we need to list them. It would just mean that if another method is added at a later time, the docs would be wrong. So we should just say that someone needs to implement the interface and not be more specific.

So...

1. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -12,10 +12,10 @@
   + * example). The only methods this service needs to implement are applies and
   + * determineActiveTheme.
   

   So please leave out the "The only methods..." sentence. Replace it by saying that your service class needs to implement this interface.

2. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -46,7 +46,8 @@ public function applies(RouteMatchInterface $route_match);
   +   *   default ,one should kick in instead.
   

   comma should be after "one" not before.

comma should be after "one" not before.

Upps... sorry.

I hope this is more correct.

Thanks!

+++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
@@ -46,7 +45,8 @@ public function applies(RouteMatchInterface $route_match);
+   *   default one, should kick in instead.

Can we use something other than 'kick in' that would be friendlier for people with English as an additional language?

Hi Catch,
I have recreated the patch #10. Done the changes mentioned in #12

Thanks for the new patch! Still needs a bit of work though:

1. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -12,10 +12,9 @@
   + * To set the active theme, create a new service tagged with 'theme_negotiator',
   

   There should not be a comma at the end of this line. Sorry for not noticing this earlier.

2. +++ b/core/lib/Drupal/Core/Theme/ThemeNegotiatorInterface.php
   @@ -46,7 +45,8 @@ public function applies(RouteMatchInterface $route_match);
   +   *   The name of the theme, or NULL if other negotiators, like the configured
   +   *   default one, should commit instead.
   

   I think that "commit" isn't the right word here.

   Maybe "be used" would be better?

Updated patch from #13 to include suggestion from #14

@zaurav , I am sorry !! I didn't refresh the page before uploading the patch. Please ignore mine. Thanks

These patches seem to be about the same. Either one is fine.

Setting version to 8.2.x so they get tested against that.

I have put the test against 8.2.x
(Patch on #16)

Thanks. Just as a note, I didn't bother to do that, because all RTBC issues are automatically tested against the version of the issue, about once every 24 hours. So I think at the first retest it would have moved to 8.2.x.

Upps! I did not know that.
Thanks.

Committed 9fb6ec3 and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!