Make theme negotiator works with other entity types

issue title should include what this is about (diff display theme). also, you can close the other issue (fixed)

+++ b/src/VisualDiffThemeNegotiator.php
@@ -16,7 +16,7 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
+    if ($route_match->getRouteName() === 'diff.revisions_diff' || $this->checkRouteName($route_match)) {

You put some check into "checkRouteName" that is a function name that doesn't tell me anything about what it checks.

Better Outwourcing would be a method isDiffRoute() that checks both diff.revisions_diff route name and the regular expression match.

+++ b/src/VisualDiffThemeNegotiator.php
@@ -16,7 +16,7 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
       if ($route_match->getParameter('filter') === 'visual_inline') {

This check is the fastest. Start with this and in 99% of the cases all the other checks are not made.

1. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -16,13 +16,13 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
      public function applies(RouteMatchInterface $route_match) {
   ...
          if ($route_match->getParameter('filter') === 'visual_inline') {
   

   Wrong indentation.

2. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -16,13 +16,13 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +        if ($this->isDifRoute($route_match)) {
   

   Typo

+++ b/src/VisualDiffThemeNegotiator.php
@@ -33,4 +33,19 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
+   * Check diff route.
...
+   * @return int
+   *   Returns 1 if route name is 'diff.revisions_diff' for node or start with
+   *   'entity.' and ending with '.revisions_diff'.

@return Should be boolean and not explain details.

The summary "Check diff route" is too short and doesn't explain what it means.

1. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -33,4 +34,19 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +   * Check if route name is 'diff.revisions_diff' for node or if route name
   +   * starts with 'entity.'  and ending with '.revisions_diff' for other entity.
   

   The summary needs to be one line.

   See doc standard how to add more detail.

2. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -33,4 +34,19 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +   * @return boolean
   

   See coding standards:
   bool (NOT "boolean" or "Boolean"). If only TRUE or only FALSE is a possible value, rather than either one being possible, use true or false instead of bool.

3. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -33,4 +34,19 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +    $regex_pattern = '/^entity\..*\.revisions_diff$/';
   ...
   +      preg_match($regex_pattern, $route_match->getRouteName());
   

   The integration test would be hard to test (dependency to entity module, other entity type, ...), but our unit test is easy to extend to trigger this case. We really need test coverage for bugs!

Now the indentation is wrong in the comment.

The isDiffRoute covers two cases.
Your Unit test need to test both cases!

Committed, thx.

Noooo, I should make this comment first!

1. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -15,15 +15,16 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +    if($routeMatch->getParameter('filter') === 'visual_inline') {
   +      if($this->isDiffRoute($routeMatch)) {
   +        if($this->configFactory->get('diff.settings')->get('general_settings.visual_inline_theme') === 'standard') {
   

   Space missing between "if" and "(".

2. +++ b/src/VisualDiffThemeNegotiator.php
   @@ -15,15 +15,16 @@ class VisualDiffThemeNegotiator extends AdminNegotiator {
   +    return;
   

   Why don't we return FALSE anymore?

I would like to see if this passes tests first.

Committed.