Create an interface for the public methods on ExposedFormPluginBase

This is the first issue I am trying to fix. So, can you clarify what an interface is and maybe briefly explain what needs to be done?

This does not seem like a real Novice project, actually.

There are different levels of novice, for sure. This would have been good for a PHP dev who is new to Drupal.

Still needs some work for the docblocks, those are marked with @todo.

Thanks! This is a great start, and perhaps it is now a Novice project if you don't want to continue with it. :)

Things that need to be fixed/finished to finalize this:

1. The @todo sections in the new Interface class

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php
   @@ -15,31 +15,18 @@
   - * @defgroup views_exposed_form_plugins Views exposed form plugins
   

   Moving the @defgroup to the Interface file is a good idea. However, I think that we should then have

   @ingroup views_exposed_form_plugins

   added to the base class doc header, so that this base class still shows up on the Topic page.

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,115 @@
   + * Exposed form plugins implement
   + * \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface. They must
   + * be annotated with \Drupal\views\Annotation\ViewsExposedForm annotation,
   + * and they must be in namespace directory Plugin\views\exposed_form.
   

   We lost the information that was previously in this paragraph about the existence of the base class. Let's add that back in. There are other plugins with base classes you can look at for a model.. let's see... Here's the Block API topic, which mentions both the Block interface and the Block base plugin class.
   https://api.drupal.org/api/drupal/core!modules!block!block.api.php/group...

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,115 @@
   +   * Render the exposed filter form.
   

   Render -> Renders

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,115 @@
   +   *   Nested array of keys to exclude of insert into $view->exposed_raw_input.
   

   This line I think needs some attention... doesn't make sense... seems garbled.

I think this should be merged with #2640740: Fix docblocks in file ExposedFormPluginBase.php in order to address #7

Should this be a single interface, or should preRender / postRender / preExecute / postExecute be on their own, since other classes implement these (though with different arguments in a few cases)?

@mpdonadio these specific methods with those params are unique to exposed forms, this is fine as is.

Thanks! This is getting better.... It needs a bit of work here and there:

1. +++ b/core/modules/views/src/Annotation/ViewsExposedForm.php
   @@ -10,7 +10,7 @@
   - * @see \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase
   + * @see \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface
   

   Adding an @see here to the new interface is good. But let's also keep the previous @see, because the base class is still very useful.

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php
   @@ -15,31 +15,20 @@
   + * @ingroup views_exposed_form_plugins
   

   Thanks for adding the @ingroup here. However, it needs to go at the bottom of the doc block, not at the top.

   See https://www.drupal.org/node/1354#order

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   + * Exposed form plugins implement
   + * \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface. They must
   + * be annotated with \Drupal\views\Annotation\ViewsExposedForm annotation,
   + * and they must be in namespace directory Plugin\views\exposed_form.
   

   Again: can we please add information here about the base class? I asked for this in my last review. See above comment, item 3 in comment #7.

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   * Does some changes before rendering the exposed form
   

   This is a bit vague. Changes in what? I think all the pre/post functions maybe need a bit more explanation?

   The first line here needs to end in a . also.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   * $param array $values
   +   */
   

   Any @param needs a documentation line below it.

   See
   https://www.drupal.org/node/1354#param

6. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   */
   +  public function postRender(&$output);
   

   Needs @param docs.

7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   * Does some changes before the actual query gets executed.
   

   I'm not sure we need "actual" in here?

8. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   * Does some changes before the actual query gets executed.
   +   */
   +  public function postExecute();
   

   before => after

   Also not sure we need "actual" in here?

9. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,120 @@
   +   * Submits views exposed form.
   

   views -> the view

   At least, all of the other functions refer to it that way.

   Actually... Maybe we should make them all just say "the exposed form" and leave out the word "view" or "views"? I think that would be a bit more concise, and no less clear, since this is an interface for Views anyway.

10. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,120 @@
    +   * Called when the "Reset" button is triggered.
    

    First line docs for functions need to start with a verb. See
    https://www.drupal.org/node/1354#functions

11. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,120 @@
    +   * The method resets all states of exposed form: user inputs,
    

    of exposed => of the exposed

12. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,120 @@
    +   * stored session and the form state.
    

    needs comma before "and"

13. Item 5 of comment #7 still is a problem. Please fix.

+++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
@@ -0,0 +1,141 @@
+   *
+   * This is meant to be used mainly to deal with the handlers whose output
+   * cannot be cached at row level but can be cached at display level. The
+   * typical example is the row counter.
+   *
+   * @param $output
+   *   The field rendered output.
+   *
+   * @return string[]
+   *   An associative array of post-render token values keyed by placeholder.
+   */

This documentation is wrong. You just copied it from some other place, but postRender() is simply a method which is called after rendering was done. There are not field stuff involved here, see core/modules/views/src/ViewExecutable.php:1435

Your interdiff file is also empty, which makes it difficult to see what you changed. Please make a new patch and an interdiff back to the patch in #12. Thanks!

Please go back and read my review in #13. Most of this is repeated from that and previous reviews, which have STILL not been addressed.

In the future, please do NOT make patches without addressing previous reviews. It wastes reviewers time if you only address part of the reviews or completely ignore them. Thanks.

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   + * Exposed form plugins implement
   + * \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface. They must
   + * be annotated with \Drupal\views\Annotation\ViewsExposedForm annotation,
   + * and they must be in namespace directory Plugin\views\exposed_form.
   

   Sigh. I think I have made this comment multiple times now.

   CAN WE PLEASE also have a mention in this paragraph of the base class and that being a useful starting point?

   See my previous reviews for more thoughts on this.

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   + * @see  \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface
   + * @see \Drupal\views\Annotation\ViewsExposedForm annotation
   + * @see  \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase
   

   These three lines should all line up -- there are extra spaces between @see and the first \ in two of them.

   Also the second one has a stray word "annotation" at the end.

   But really.. We don't need the @see lines here at all, since the previous paragraph links to all of these, and on api.drupal.org, presumably they will all be listed automagically by having @ingroup in each of the classes (except the one that is in this file, which is automatically included by being within the @{ @} brackets).

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   + * Interface for the exposed filter form plugins.
   

   I don't think "the" is good here.

   Also, this should probably have a bit more documentation. It doesn't even say it's for Views for instance, or tell what exposed filter forms are?

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   * This actually does more than that; because it's using FAPI, the form will
   +   * also assign data to the appropriate handlers for use in building the
   +   * query.
   

   I don't think this paragraph makes sense. The use of FAPI doesn't imply that a form does anything with handlers.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   *   Determines whether the exposed form rendered as part of a block or not.
   

   rendered => is being rendered

   (??)

6. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   *   Form build array or an empty array.
   

   Under what circumstances does it return an empty array? This needs to be documented.

7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   * This gives the handlers some time to set up before any handler has
   +   * been rendered.
   

   What handlers are we talking about here? This is confusing.

8. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   * @param string $output
   +   *   The output of the rendered view.
   

   Really I do not think this is a string? It is most likely a render array?

9. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,134 @@
   +   * Does some changes before the query gets executed.
   +   */
   +  public function preExecute();
   +
   +  /**
   +   * Does some changes after the query gets executed.
   

   uh, these are vague.

10. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,134 @@
    +   * Resets all the states of the exposed form: user inputs, stored session, and the form state.
    

    This line is too long. As it is the first line of a function, it needs to be shortened to less than 80 characters and not wrapped.

    Probably the thing to do is drop the part after the : and move that to a second sentence in the next paragraph instead.

@jhodgdon I think I addressed most of the comments you added on #19, please let me know if I forgot someone.

I was one of the developers who originally wrote the ExposedForms plugins so I made some modifications to some of the doc strings. I think the makes sense, but please let me know if they have some wording issues.

Thanks! Reviewing the interdiff... still a lot that isn't right or wasn't addressed, sigh.

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -25,37 +25,33 @@
     * and they must be in namespace directory Plugin\views\exposed_form.
   

   See item 1 in my previous review. Still not fixed.

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -25,37 +25,33 @@
   - * @see  \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface
   - * @see \Drupal\views\Annotation\ViewsExposedForm annotation
   - * @see  \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase
   + * @see \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase
   + * @see \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface
   

   So... In my previous review, I suggested removing all of these @see lines, since they're all mentioned in the previous paragraph. Or keeping them all but cleaning them up.

   Instead, only one line was removed. I don't understand this. We should either remove all of them or leave all of them. And if we're leaving them (which is OK), remove the stray word 'annotation' at the end of the second one. Not remove the entire line.

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -25,37 +25,33 @@
   + * Interface for exposed filter form plugins.
   

   I asked in my last review for more information to be added to this interface doc block. None was added. Please add more information.

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -25,37 +25,33 @@
       * Runs before the exposed form is rendered.
       *
   -   * This gives the handlers some time to set up before any handler has
   -   * been rendered.
   +   * This gives exposed forms some time to set up before any field has been
   +   * rendered.
   

   This still doesn't make sense.

   The first line says "Runs before the exposed form is rendered".

   The next paragraph says it gives exposed forms more time to do something before fields are rendered... but ... huh? This says it is running before the exposed form is rendered, so how is that related to field rendering? I don't understand it.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -78,12 +74,14 @@ public function preRender($values);
      /**
   -   * Does some changes before the query gets executed.
   +   * Give the opportunity to perform some extra operation before the query gets
   +   * executed.
   

   First line of doc block needs to be a one-line summary, ending at 80 characters with a ., not a multi-line summary.

   Also "perform some extra operation"... maybe "operations"? And... I don't really think this is much less vague than what was already there. Maybe say something like:

   Implement if your exposed form needs to run code before query execution.

   or something similar?

6. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -78,12 +74,14 @@ public function preRender($values);
   +   * Give the opportunity to perform some extra operation after the query gets
   +   * executed.
   

   See previous item.

7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -123,7 +121,8 @@ public function exposedFormValidate(&$form, FormStateInterface $form_state);
      /**
   -   * Resets all the states of the exposed form: user inputs, stored session, and the form state.
   +   * Resets all the states of the exposed form:
   +   * user inputs, stored session, and the form state.
   

   Again, this needs to be a one-line summary. Wrapping it to two lines is not OK.

8. Item #8 from my last review wasn't addressed at all as far as I can tell.

@jhodgdon Thanks for your review.

Hm it seems I didn't spend enough time to fix all the items, sorry about that.. I will be working on this more deeply during this week to provide a complete patch on Thursday.

@jhodgdon I rewiewed with more details all your suggestions made on #13, #19 and 24. I'm providing several interdiff to make easier to review.

Item #8 from my last review wasn't addressed at all as far as I can tell.

Hm, I checked, maybe you meant item #10?

Altough I checked that all the docblocks start with a single line explaning what the method does, I'm still not sure if preExecute, postExecute, preRender methods etc... have the right description, after all they are use to do "something" at different steps of the view render/execution. Maybe we could link to: ViewExecutable::build() which actually call most of those methods, what do you think?

Thanks for the new patch! This time I started over and reviewed the entire patch, as the interdiffs were confusing. ;)

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   + * of exposed forms, and may add additional form elements. This plugins also
   + * allows to alter the view query based on the input selected by the user
   + * on exposed forms. See \Drupal\views\Plugin\views\exposed_form\InputRequired
   

   This sentence is ungrammatical.

   Better:

   These plugins can also alter the view query.

   [I don't think we need the "based on the input..." part.]

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   + * The default exposed filter used by views is implemented on
   + * \Drupal\views\Plugin\views\exposed_form\Basic and can be extended to override
   + * functionality or replaced completly to achieve other kind of behaviour.
   

   Actually I don't understand what this paragraph is trying to say. How would you override it really? I mean, if you make a class that extends Basic, how would Views know to use your version? Maybe just leave this out?

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   + * functionality or replaced completly to achieve other kind of behaviour.
   

   If we're keeping this...

   completely is misspelled

   Also behavior should not have a u in it (we use American English spellings in the project).

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   * @param bool $block
   +   *   Determines whether the exposed form is being rendered as part of a block
   +   *   or not.
   

   A parameter doesn't really "determine" things. Also this is optional.

   How about:

   (optional) TRUE if the exposed form is being rendered as part of a block; FALSE (default) if not.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   * @return array
   +   *   Form build array.
   

   Looking at the code in the base class, I think we should also say that this method returns an empty array if the form is being rendered as a block.

6. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   * This gives exposed forms some time to set up before any field has been
   +   * rendered.
   

   Can we use an "Implement if..." paragraph here, like the other pre/post methods have now?

7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   * Implement if your exposed form needs to run code after the view was
   +   * rendered.
   

   This is nice.

8. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   * @param $output
   

   What type is this? We should always have a type in a @param line.

9. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,155 @@
   +   *   The field rendered output.
   

   how about "rendered field output" instead of "field rendered output"?

10. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,155 @@
    +   * @return string[]
    +   *   An associative array of post-render token values keyed by placeholder.
    +   */
    +  public function postRender(&$output);
    

    That's rather odd. It looks like postRender() takes $output by reference, meaning presumably that it can be altered by the function. Does this really have a return value? Looking in ViewExecutable where this is called, I don't see it using a return value, so I suspect it doesn't have one.

11. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,155 @@
    +   * Give the opportunity to perform some extra operation after the query gets
    +   * executed.
    

    How about changing this to the "Implement if..." wording too?

12. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,155 @@
    +   * @see \Drupal\views\Form\ViewsExposedForm
    

    This @see seems like it might be useful for exposedFormAlter() and exposedFormValidate() also.

    Anyway... all three of these methods are kind of confusing. There's this method renderExposedForm()... I guess I don't understand, from the documentation here, how these three methods are related to the form returned by that method, if they are at all, or is it a different form? or what? Very confusing. It especially doesn't make sense that a plugin that is returning a form would also need to alter it itself? Needs explanation.

13. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
    @@ -0,0 +1,155 @@
    +   * This method is called when the "Reset" button is triggered and clears:
    +   * user inputs, stored session, and the form state.
    

    We don't really need a : at the end of this line.

Thanks!

how about "rendered field output" instead of "field rendered output"?

I replaced this with the docs of the original code on ViewsExcutable.

That's rather odd. It looks like postRender() takes $output by reference, meaning presumably that it can be altered by the function. Does this really have a return value? Looking in ViewExecutable where this is called, I don't see it using a return value, so I suspect it doesn't have one.

Agree, deleted.

Anyway... all three of these methods are kind of confusing. There's this method renderExposedForm()... I guess I don't understand, from the documentation here, how these three methods are related to the form returned by that method, if they are at all, or is it a different form? or what? Very confusing. It especially doesn't make sense that a plugin that is returning a form would also need to alter it itself? Needs explanation.

I see what you mean. So basically there are two steps when building the exposed forms. The first step request all the widgets to all the filters and sorts handlers configured to be exposed, and the second one is like a final alter method that can see all the attached widgets. I included some extra docs to clarify that. I hope now be more clear.

Great! This is getting much better.

A few minor things remaining:

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,161 @@
   + * of exposed forms, and may add additional form elements. This plugins can
   

   This plugins => These plugins

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,161 @@
   + * The default exposed filter used by views is implemented on
   + * \Drupal\views\Plugin\views\exposed_form\Basic and can be extended to override
   + * functionality or replaced completely by extending
   + * \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase instead
   + * to achieve other kind of behavior.
   

   Again, this needs more explanation if it's going to stay in this patch. I don't think that if you just make a new exposed form class it will be used by a view.

   And... really what is this trying to say? Let's just leave it out. I don't think it provides any useful information that is not in the first paragraph? If it does, I am not understanding it.

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,161 @@
   +   *   @see \Drupal\views\ViewExecutable::render
   

   This @see line should be justified left (it has two spaces before the @see now), and also you need a blank line between the @param docs and the @see.

   Also when linking to a method, add () after the method name:

   ... ::render()

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,161 @@
   +   * After each exposed handler added its own widget, this method is
   +   * called to modify the final form array.
   

   This sentence is mixing up present and past tense.

   And... Oh, I finally get why this method exists. Thanks!

   So can I suggest something more like this maybe:

   The exposed form is built by calling the renderExposedForm() method on this class, and then letting each exposed filter and sort handler add widgets to the form. After that is finished, this method is called to let the class alter the finished form.

   Or something like that?

   Then it would be useful if this alter method and the render method have @see lines linking to each other, I think?

Thanks @jhodgdon. Addressed Items 1 to 4 on this patch.

I just discovered two more things. First, resetForm should not be part of the interface since it is called from the ExposedFormPluginBase class, so force other plugins to implement this method should not be neccessary (the exposed form could event not have a reset button as a UI option). I documented the resetForm method on the base class and removed the method from the interface.

And second, the postExecute method is never actually called (This is the reasone becuase there is no @see on that docblock). I'll open another issue to address that, for now I just defined the method on the interface.

Unrelated test failures.

This is getting better, but is still not quite ready:

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   */
   +  public function renderExposedForm($block = FALSE);
   

   Again, can we please have an @see that links this method to the alter form method on the interface?

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   * Implement if your exposed form needs to run code before the view is being
   +   * rendered.
   

   Let's take out the word "being" here.

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   * Implement if your exposed form needs to run code after the view was
   

   was -> is

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   * @param string $output
   +   *   The rendered output of the view display.
   

   Question: Is this really a string, and not a render array? Because if it is, it would be really really really difficult to alter, and it seems pointless to pass it as an argument to this function.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   */
   +  public function postExecute();
   

   Does it make sense to have the same @see line added here as there is in the preExcecute() method?

6. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   * @see \Drupal\views\Form\ViewsExposedForm::buildForm()
   +   */
   +  public function exposedFormAlter(&$form, FormStateInterface $form_state);
   

   Again, can we also have an @see that points to the renderExposedForm on this interface?

7. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   *   Array of keys which will not appear in $view->exposed_raw_input.
   

   grammar nitpick:

   which -> that

8. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,153 @@
   +   *   Array of keys which will not appear in $view->exposed_raw_input.
   +   *   For example, 'form_build_id'.
   

   Also please rewrap these two lines into a paragraph. The first line is pretty short. And the second sentence isn't really a sentence, so how about fixing that too? Could say:

   ... exposed_raw_input; for example, 'form_build_id'.

Thanks! Items 1, 2, 3, 6, 7 and 8 fixed on this patch.

Regarding 4:

Question: Is this really a string, and not a render array?

Yes it is an string. The pager plugin is also receiving this as an string, I think is better to open another issue if we want to change this on both plugins.

Regarding 5:

Does it make sense to have the same @see line added here as there is in the preExcecute() method?

I opened #2679493: postExecute() is never called on exposed form plugins since this method is not called anywhere.

I have no energy for reviewing this patch any more, sorry. Let's move it into the Views component and let the views maintainers deal with it.

@jhodgdon Thanks for your help! @dawehner please let me know if you need anyting else here.

Rerolled for 8.2.x.

I took a quick look over this patch. There are a few small grammar/punctuation things that could be improved:

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php
   @@ -305,6 +298,17 @@ public function exposedFormSubmit(&$form, FormStateInterface $form_state, &$excl
   +   * This method is called when the "Reset" button is triggered and clears
   +   * user inputs, stored session, and the form state.
   

   Consider making this into two sentences, ending the first one after "is triggered", for clarity? Or at least adding some punctuation there.

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,156 @@
   + * - Usually you will want to extend
   + *   \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase class, which
   + *   provides a common utility methods.
   

   "extend Xyz class" either needs to be "extend the Xyz class" or just "extend Xyz" (without the word "class"). As it is, it is not grammatical.

   I am not actually sure I agree that the base class "provides common utility methods". What it does is provide a base implementation of many/all of the methods in the interface (I'm not sure if it's many or all).

   But really, people reading this documentation probably understand what "base class" means, so you could probably just leave out the whole "which" clause here.

3. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,156 @@
   + * - Exposed form plugins use the annotations defined by
   + *   \Drupal\views\Annotation\ViewsExposedForm. See the
   + *   @link annotation Annotations topic @endlink for more information about
   + *   annotations.
   

   "use the annotations defined by Xyz" => "are annotated with Xyz annotation".

4. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,156 @@
   +interface ExposedFormPluginInterface extends ViewsPluginInterface {
   

   I think that this interface's doc block should have @see links to both the base class and the annoation.

5. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,156 @@
   +   *   Array of keys that will not appear in $view->exposed_raw_input; For
   

   For should be lower-case

Thanks @jhodgdon. I think this should achieve all your comments.

Patch doesn't apply, apparently.

Sorry, accidentaly a temporal file was included in the patch.

Looking very good! I found just two small things to address, introduced by the latest changes:

1. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php
   @@ -305,6 +298,17 @@ public function exposedFormSubmit(&$form, FormStateInterface $form_state, &$excl
   +   * This method is called when the "Reset" button is triggered. Also clears
   +   * user inputs, stored session, and the form state.
   

   Did this say "Also" before? Ah, no, that was one sentence before joined by and... So. It isn't an "also", is it? I think this (now separate second) sentence is just explaining what it does.

   So I suggest changing "Also" to "In the base class, it"

2. +++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
   @@ -0,0 +1,158 @@
   + *   \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase class
   

   This should end in .

Thanks

OK, I think this is fine now. Thanks!

diff --git a/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
index 6de8fc2..9759ae5 100644
--- a/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
+++ b/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginInterface.php
@@ -1,10 +1,5 @@
 <?php
 
-/**
- * @file
- * Contains \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginInterface.
- */
-
 namespace Drupal\views\Plugin\views\exposed_form;
 
 use Drupal\Core\Form\FormStateInterface;

Fixed on commit - @file is no longer required in PSR0/4 classes.

Committed 567b140 and pushed to 8.2.x. Thanks!