Documentation fixes for DisplayPluginBase.php

Thanks! The changes here look pretty good. There are a few more things you could fix up:

a)

-            // one is all we need; if we find it, return true.
+            // One is all we need; if we find it, return true.

true -> TRUE

b)

/**
-   * Check to see if the display can put the exposed formin a block.
+   * Check to see if the display can put the exposed form in a block.

Verbs: Check -> Checks [throughout the file, it looks like all the methods have the wrong verbs]

c)

-            // include non-engine theme files
+            // include non-engine theme files.

In this one place, you forgot to capitalize Include.

d)

-   *   Select the new state of the option.
+   *   Select the new state of the option:
    *     - TRUE: Revert to default.
    *     - FALSE: Mark it as overridden.

You could also clean up the list indentation -- see http://drupal.org/node/1354#lists -- several places in the file need this fixed.

e)

/**
-   * Not all display plugins will support filtering
+   * Not all display plugins will support filtering.
    *
    * @todo this doesn't seems to be used
    */
   public function renderFilters() { }

This function description does not meet our standard of describing what the function actually does (see http://drupal.org/node/1354#functions). The next one suffers from the same problem... The @todo here is also grammatically awful.

I guess I missed this one:

/**
    * Determine if this display is the 'default' display which contains
-   * fallback settings
+   * fallback settings.
    */
   public function isDefaultDisplay() { return FALSE; }

All functions/methods need to start with a *one-line* description of what the function does, starting with a third-person verb (Determines...).

Here's another one that needs fixing:

/**
-   * Check to see which display to use when creating links within
+   * Checks to see which display to use when creating links within
    * a view using this display.
    */
   public function getLinkDisplay() {

Etc.

Lets postpone this 'til after feature freeze.

active?

This is not required to be frozen anymore afaik.

#3 rerolled, there is still correct problems in #4.

Looks pretty good, just a couple of things I noticed:

1. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2035,9 +2037,8 @@ public function optionsOverride($form, FormStateInterface $form_state) {
   +   *   (optional) TRUE to revert new state option to default, FALSE to mark it
   +   *   as overridden. Defaults to NULL.
   

   What does NULL do?

2. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2076,21 +2077,23 @@ public function query() {
   +   * Return a rendered pager for the display.
   +   *
   +   * @return bool
   

   How is a rendered pager a boolean?

In both cases though, these are about information that is missing in the docs as they currently stand, and this issue seems to be about fixing the formatting.

Digging around for the missing info will hold this issue up even further, so I'm going to set this to RTBC and suggest these are tackled as follow-ups.

fixing what was said in #4

Regarding #4, the other problem is that the descriptions at the tops of functions are supposed to be ***only one line***. They still aren't. The verbs are better but this needs to be fixed. If you need more than one line of description, put it in a separate paragraph.

Sorry, but this is not ready to commit.

Trying to combine all the comments.
To split the description in one line, I'm rephrased it a bit. This is acceptable?

Also noticed in optionLink() : a title to anchor is wrapped in <span>. Is it not a bug?

Yes, that's the idea, thanks!

Most of these splits are great. I think we could use a few small adjustments to what you did, and I read through the whole patch so some of the items below are not related to the most recent changes, but they all need to be fixed in this patch:

a)

+   * Determines if this display uses exposed filters.
+   * 
+   * This will let the view know whether or not to build exposed filters.
    */
   public function usesExposed() {

I don't really think we need that second line. It doesn't tell us much/anything. Let's take it out to reduce clutter.

b) Same in this one:

+   * Determines if this display should display the exposed filters widgets.
+   *
+   * This will let the view know whether or not to render them. 
    *

Yeah, I mean, if the answer is no, they shouldn't be displayed, then ... duh. :)

c)

+   * Checks to see which display to use.
+   *
+   * Returns display to use when creating links within a view using
+   * this display.
    */
   public function getLinkDisplay() {

This is OK, but maybe we can make it more compact, and ... looking at the code, I think it should be:

Returns the ID of the display to use when making links.

d)

+   * Determines if a option is set to use the default or current display.
    *
    * @return
-   *   TRUE for the default display
+   *   TRUE for the default display.
    */
   public function isDefaulted($option) {

First line: should be "an option".

e)

-   * Intelligently get an option either from this display or from the
-   * default display, if directed to do so.
+   * Returns an option either, if directed to do so.
+   *
+   * From this display or from the default display.
    */
   public function getOption($option) {

This doesn't actually make sense... How about:
* Gets an option, from this display or the default display.
and get rid of the 2nd line?

f) The setOption function needs a similar fix to (e)

g)

+  * Returns link to the received section.
+  *
+  * Because forms may be split up into sections, this provides an easy URL 
+  * to exactly the right section. Don't override this.
+  */
   public function optionLink($text, $section, $class = '', $title = '') {

The first line should say "Returns a link to a section of a form." I think?

h)

  /**
-   * Not all display plugins will support filtering
+   * Not all display plugins will support filtering.
    *
-   * @todo this doesn't seems to be used
+   * @todo This function no longer seems to be use.
    */
   public function renderFilters() { }

In the To Do, the last word should be "used" not "use". Also... the first line should probably be changed to something with a verb in it... perhaps "Does nothing (obsolete function)."?

i)

   /**
-   * Not all display plugins will suppert pager rendering.
+   * Returns a rendered pager for the display.
    */
   public function renderPager() {
     return TRUE;

That cannot be accurate. The function returns TRUE. That is not a rendered pager, it is a Boolean value... I do not know what the function does but this description isn't right.

j)

   /**
-   * When used externally, this is how a view gets run and returns
-   * data in the format required.
+   * Used externally, to view get run and returns data in the format required.
    *
    * The base class cannot be executed.
    */

Um. The new version is not grammatical and should probably be:

Executes the view and returns data in the format required.

Thank you for your corrections!
For renderPager(), maybe should be "Checks to see if the display plugins support pager rendering"?

If I overlooked something please let me know.

An interdiff would have been helpful! See https://drupal.org/documentation/git/interdiff -- here's one for anyone else reviewing (interdiff is between patch in #15 to #17).

So... All this looks great except the setOption method:

+   * Sets an option, from this display or the default display.
    */
   public function setOption($option, $value) {

That "from" should be "on", for the set function (it is "from" on the get function, but that doesn't make sense for set).

I like your wording for the renderPager function -- good.

So, just one small change (please make an interdiff file) and it should be good! Thanks!

Thank you again for your attentiveness.

I guess we need a reroll after #2394041: Row language settings from entity views should be on display level for all views.
Rerolled #17 w/o changes.

Attaching the #18 correction and interdiff. (It's my first interdiff so I hope I created it right)

Nice job! Looks good to me. Thanks!

I'm trying to add a beta phase evaluation to the issue summary.

The documentation changes seems to be fine here. Thank you for your work!

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
@@ -2614,10 +2616,15 @@ public function getArgumentText() {
+   *   element:
+   *   - items per page title: The title text for the items_per_page form
+   *     element.
+   *   - items per page description: The description text for the
+   *     items_per_page form element.

Should we maybe quote the keys which have spaces?

Did you mean it should be something like that?

Quoting keys is not what our documentation standards for lists say to do. :( We should go back to the patch in #19 I think. Also, **please** when you upload a new patch, also include an interdiff!

I created #24 only to clarify the quoting suggestion. So I thought that the interdiff is not required. Sorry.

patch 19 is passed now, return to RTBC.

I am re-uploading the patch in #19 so that the wrong patch does not get committed by mistake.

1. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -481,7 +478,7 @@ public function defaultableSections($section = NULL) {
   -      // These guys are special
   +      // These guys are special.
   

   This is unnecessarily gendered language. There are no guys here. These sections are special. would be fine but what would be really nice is to know why these sections are special?

2. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -1035,8 +1028,10 @@ public function overrideOption($option, $value) {
   +   * Because forms may be split up into sections, this provides an easy URL ¶
   +   * to exactly the right section. Don't override this.
   

   Space at the end of this line and the word to can be moved to the line above.

3. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2295,14 +2293,15 @@ public function access(AccountInterface $account = NULL) {
   +   * Sets up any variables on the view prior to execution. ¶
   

   Unnecessary space at the end of this line.

4. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -182,7 +182,7 @@ public function initDisplay(ViewExecutable $view, array &$display, array &$optio
   +    // Make some modifications.
   

   This is not a helpful comment.

What can we do with #30-4?
It would be acceptable if we will just specify that this modifications affect $this->options?

The comment in #4 can probably be removed. Comments should explain the why not the how/what in code. If there is no "why" to comment on, get rid of the comment.

Please review.

This addresses the review comments, thanks!

Committed 2db5da0 and pushed to 8.0.x. Thanks!