Refactor PanelizerInterface for better DX

Changing the title here because I am no longer convinced of the approach the title implies.

I've toiled on this for quite a while with little to no good result. As a by-product of this, I asked phenaproxima to look over my shoulder and see if 4 eyes and 2 minds had better luck. Phenaproxima had some great ideas about moving methods onto the EntityViewDisplay which simplified a lot of things. In the mean time, we've attempted to remove many of the NULL parameters present in PanelizerInterface methods. This makes the code a bit easier to both read and grok. It likely means we could use some caching in a couple places and that we should definitely nail down render caching, but the result is code which is starting to get easier to read and follow which I think (and hope) was the spirit of this issue.

Since we're beginning to make some head way on this, I wanted to post a patch for others to see and begin giving feedback on. This won't pass tests, but I'd like to see what testbot says all the same, so NRing.

Eclipse

Ok, I believe this will pass tests, continuing to refactor from this.

Eclipse

Asking for dsnopek to take a look.

@EclipseGc: Are you finished with your refactoring?

A minor reroll.

I believe "needs work" sums up EclipseGc's comment in #10.

Bumping the issue to see if dsnopek can give this a review.

From this week's scotch meeting:

damienmckenna:
@eclipsegc: Based upon what you've discussed with David already, do you have any idea on what his reservations might be? (edited)

eclipsegc:
he had a different approach in mind. I attempted (my best interpretation of) it and failed. Phenaproxima and I spent some time looking at it and came up with a different approach
I think david just wanted to try what he had in mind and also fail (or succeed) before he weighed in on a completely different approach
had he succeeded we’d have probably weighed the two against each other
and had he failed we’d have probably had a green light

japerry:
if you and Adam feel fairly confident about your approach we should just go with it

eclipsegc:
but since his time constraints are preventing him from trying...
well, we could assign review of what we’ve done thus far to someone else and get a “yeah this is sane” or whatever still before we continue
@damienmckenna I think david’s main reservation is that it was not at all what he was contemplating doing
they were radically different approaches to simplifying the DX of that interface and class

damienmckenna:
@eclipsegc: You said you failed to complete an attempt on his idea, do you think if you worked with Phenaproxima a little you might be able to complete it?

eclipsegc:
@damienmckenna that’s exactly why I brought him in on it was my continued failure. We worked together, felt it was unworkable and changed direction
@damienmckenna the class is very confusing as you read it about when it’s creating a thing vs loading a thing
@damienmckenna and david would like us to pass fewer arguments, which meant adding a dumb data object to hold the existing arguments which made the dividing line between load/create even blurrier
@damienmckenna Phenaproxima and I opted instead to stop passing NULL display objects in the arguments and to load them much more clearly (which means we’ll need a static caching layer probably) but makes the code flow much more obvious. The code today is confusing because it’s basically like “If you have a display object pass it, if you don’t I’ll figure out how to work anyway) which means all the methods are mixed mode.

We (japerry and I) agreed to let EclipseGc continue on the path he was currently taking.

Assigning this issue to EclipseGc.

1. +++ b/panelizer.module
   @@ -88,6 +88,11 @@ function panelizer_entity_type_alter(array &$entity_types) {
   +  $entity_types['entity_view_display']->setClass('\Drupal\panelizer\PanelizerEntityViewDisplay');
   

   why not PEVD:class?

2. +++ b/src/PanelizerEntityViewDisplayInterface.php
   @@ -0,0 +1,33 @@
   +interface PanelizerEntityViewDisplayInterface {
   +
   +  public function isPanelized();
   

   would be great to document them

@andypost: I think the goal is to polish it off once the decision was made what direction to take.

Additional follow-up from dsnopek:

"So, it's replacing a core class (the one for entity view displays), which is something that can only be done once on a Drupal site ever and has the chance of conflicting with other modules which otherwise could work together fine. Adding a new class to wrap the core class should also work, and adding more classes really doesn't hurt anyone. Kris said he tried that approach but hit problems? Anyway, that's the basic summary"

Both EclipseGc and tim.plunkett have a plan to work around this correctly, quoting tim.plunkett:

so what dsnopek wants should be 100% achievable, just a few extra steps needed
so i didn't have an opinion before, i do now: you shouldn't override, you should duplicate/clone

This won't go in 3.x.

Marking this Postponed until 3.0 is out.