Explain fields configuration

Added description to the diff plugins and displayed them on the settings page. I guess that the description after the "Layout" table (marked in red in the screenshot) now could be removed since then it is kinda repeating the description provided for each of these rows.

Anyway, any improvements about the descriptions ?

1. +++ b/src/Form/GeneralSettingsForm.php
   @@ -121,6 +121,7 @@ class GeneralSettingsForm extends ConfigFormBase {
   +        '#description' => $this->diffLayoutManager->getDefinition($id)['description'],
   

   I like this approach but I think it might be better to fallback in something when the description is not defined.

2. +++ b/src/Plugin/diff/Layout/SplitFieldsDiffLayout.php
   @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
   + *   description = @Translation("Split fields plugin compares revisions side by side. This plugin is configurable."),
   
   +++ b/src/Plugin/diff/Layout/UnifiedFieldsDiffLayout.php
   @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
   + *   description = @Translation("Unified fields plugin compares revisions based on the GitHub layout. This plugin is configurable."),
   
   +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
   @@ -21,6 +21,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
   + *   description = @Translation("Visual Inline plugin uses external libraries to compare revisions, based on the HTML Diff module. This plugin is not configurable."),
   

   And if we gonna add this, it's better to add it in diff/src/Annotation/DiffLayoutBuilder. :)

+++ b/src/Plugin/diff/Layout/SplitFieldsDiffLayout.php
@@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
+ *   description = @Translation("Split fields plugin compares revisions side by side. This plugin is configurable."),

+++ b/src/Plugin/diff/Layout/UnifiedFieldsDiffLayout.php
@@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
+ *   description = @Translation("Unified fields plugin compares revisions based on the GitHub layout. This plugin is configurable."),

+++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -21,6 +21,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
+ *   description = @Translation("Visual Inline plugin uses external libraries to compare revisions, based on the HTML Diff module. This plugin is not configurable."),

I don't think is needed to add the plugin name in the description.

Instead of "This plugin is configurable" IMHO could be better to write "Field plugin settings are applied to this layout."

Added a "Description" column. I don't get how we will use the property created in diff/src/Annotation/DiffLayoutBuilder. About the descriptions, I've used a similar template like in \Drupal\sharemessage\Form\ShareMessageForm::form() unsing the Drupal\sharemessage\Plugin\sharemessage\Addthis/OGHeaders/Sharrre plugins, where the plugin name appears again in the description. Do you still want to remove it ?

+++ b/src/Tests/AdminFormsTest.php
@@ -30,6 +30,19 @@ class AdminFormsTest extends DiffTestBase {
+    $this->assertText('Visual Inline plugin uses external libraries to compare revisions, based on the HTML Diff module. This plugin is not configurable.');

Since Visual layout cannot be installed in testbot, this line should go away.

Removed that line, not sure how to make the test to run/detect the implementation from a hook_help().

And I'm still wondering if we still need to keep that description below the table.

Added help_block in the test, edited the descriptions.
Not sure if it is fine as it is right now, how would you warn the user which layout plugins he/she can configure per field at admin/config/content/diff/fields otherwise ?

1. +++ b/diff.module
   @@ -79,7 +79,7 @@
   +      return '<p>' . t('Configurations for the revision comparison functionality and diff layout plugins.') . '</p>';
   

   "Configuration" might fit better.

2. +++ b/src/Form/GeneralSettingsForm.php
   @@ -103,7 +103,7 @@
   +      '#suffix' => '<div class="description">' . $this->t('The layout plugins that are enabled to display the diff comparison.') .'</div>',
   

   We want to avoid the diff word in the UI, better to change this to: "The layout plugins that are enabled to display the revision comparison."

3. +++ b/src/Plugin/diff/Layout/SplitFieldsDiffLayout.php
   @@ -17,7 +17,7 @@
   + *   description = @Translation("This configurable field plugin compares revisions as side by side mode."),
   
   +++ b/src/Plugin/diff/Layout/UnifiedFieldsDiffLayout.php
   @@ -17,7 +17,7 @@
   + *   description = @Translation("This configurable field plugin compares revisions as line by line mode."),
   
   +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
   @@ -25,7 +25,7 @@
   + *   description = @Translation("This field plugin uses external libraries to compare revisions. It is not configurable per field."),
   

   This area layout plugins, So change it to This configurable layout plugin... or "This layout plugin uses...

1. I think "Configurations" is fine, since there are more than one. Miro's comment at #13 suggest it as well.

2-3. done

OK, then.

Changed a bit the comments, Also added changed the settings screen to be like:

Not sure if the wording is good enough.

Yeah, these "per category settings details-wrappings" could be easier for the end user/self-explaining, but then for the visual_inline_theme aren't the label "VISUAL INLINE LAYOUT SETTINGS" and the description "Theme used for Visual inline layout when comparing revisions." too much/repetitive ?
Not sure, but imo, if we would keep it like this, I'd suggest removing the description (or at least saying something more "useful", like the Leading/Trailing examples ?)

About "Leading context lines", "Trailing context lines" and their descriptions, not sure which better wording we can use to not repeat things. Maybe just "Leading" and "Trailing" as select labels and keep the descriptions (?)

More:

1. +++ b/diff.module
   @@ -78,11 +78,14 @@ function diff_help($route_name, RouteMatchInterface $route_match) {
   +      return '<p>' . t('This table provides a summary of the field support found on the system. For every field a diff plugin can be selected and configured. This settings are applied to Unified and Split fields layouts.') . '</p>';
   

   As you said once "We want to avoid the diff word in the UI" (comment #15.2), how can we change this "For every field, a diff plugin can be selected and configured." ?

   Also imo this new help text is too long, but if it is fine, then: "This" --> "These".

2. +++ b/src/Form/GeneralSettingsForm.php
   @@ -98,11 +82,12 @@ class GeneralSettingsForm extends ConfigFormBase {
   +
   

   Unnecessary new line.

3. +++ b/src/Form/GeneralSettingsForm.php
   @@ -137,9 +127,49 @@ class GeneralSettingsForm extends ConfigFormBase {
   +    $context_lines = array(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
   ...
   +    $form['field_based_settings']['context_lines_leading'] = array(
   ...
   +    $form['field_based_settings']['context_lines_trailing'] = array(
   

   If you change/add new line/block like these kind of lines, you can directly edit the array syntax to avoid later cleanup issues and the possible "resulting conflicts".

4. +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
   @@ -25,6 +25,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
   + *   description = @Translation("Visual layout, revision comparison is made as the content is rendered for the user."),
   

   Not sure what you mean by "made as the content is rendered for the user".

Created followup to discuss about the wording stuff: #2817159: Provide better description wording for the Diff settings UI

Set back to "Fixed".