Update grammar of inline comments in \Drupal\Core\Render\Renderer::doRender

Does this text look fine? Please review.

Hi @tameeshb, I found some minor coding standard issues:

1. +++ b/core/lib/Drupal/Core/Render/Renderer.php
   @@ -320,9 +320,10 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
   +        // automatically by the Renderer.If they are missing when their
   

   Please add a space before the next sentence.

2. +++ b/core/lib/Drupal/Core/Render/Renderer.php
   @@ -320,9 +320,10 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
   +        // developer.
   

   Please transfer this to the previous line. I think this can still be inside the 80 character length limit.

Thanks!

Made changes from #3.

Made required changes

@chiranjeeb2410 ? Patch with changes was already uploaded in #5.
Also, your patch violates coding standards on line one of change.
It is recommended to upload interdiffs with patches.

Comma added to closing array item (as per code style).

I think it'd make it clearer if we clarified whether it's the keys above or below the comment block that are unsupported (and I'm not very familiar with this code yet, so not 100% sure which).

@tameeshb,

Will be sure on that part. Thanks, anyways.

@eojthebrave - As it was such a trivial change I've done it anyway - but yes, hopefully it will be obvious to most people.

The patch looks ready to RTBC to me:

- reads better than it did before
- it's comments only, so no regressions, no string, API changes, browser testing or anything
- code style, line length fine, no CodeSniffer errors, we've added the comma for final array element.

@wturell,

Changes are good to go with. Code style is fine. Inline comment issue wasn't that big a deal though. Anyways, patch applies cleanly.
Changing it to RTBC.

#11 : +1

Thanks everyone for cleaning this up!

1. --- a/core/lib/Drupal/Core/Render/Renderer.php
   +++ b/core/lib/Drupal/Core/Render/Renderer.php
   
   @@ -320,11 +320,11 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
   -        // These keys are not actually supported, but they are added automatically
   -        // by the Renderer, so we don't crash on them; them being missing when
   -        // their #lazy_builder callback is invoked won't surprise the developer.
   +        // The keys below are not actually supported, but they are added
   +        // automatically by the Renderer. If they are missing when their
   +        // #lazy_builder callback is invoked, it won't surprise the developer.
   

   I think this is definitely an improvement! However, I think it's also missing some information that is there (but not very clear) in the original text, which is "so we don't crash them". I think that might be either saying that the renderer adds them to ensure that nothing breaks when the #lazy_builder callback is invoked.

   Also let's try to get rid of the bit about surprising the developer as that's also a bit confusing.

   Maybe we should look at who originally added this documentation (Fabianx maybe?) to tease out what is meant here. Or, better, let's just have a subsystem maintainer review this and help with this. I'll ping Wim for help figuring out what this means.

2. +++ b/core/lib/Drupal/Core/Render/Renderer.php
   @@ -320,11 +320,11 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
   -        '#printed'
   +        '#printed',
   

   Technically, this is coding standards fix is out of scope, and should instead be added in an issue to add missing trailing commas to arrays across core and then enable the coder rule that checks it. See: https://www.drupal.org/core/scope#coding-standards

@xjm,

Patch uploaded with minor changes mentioned in 1. Also, +1 on the second suggestion,
regarding the coding standards fix.

@chiranjeeb2410 removed coding standard as per @xjm suggestion & re created patch. And also created issue for coding standard
#2855608: [Coding standard] Trailing comma - core/lib/Drupal/Core/Render/Renderer.php

Attached patch, interdiff files

This is correct. Thanks for fixing the language. But I have a suggestion to make it slightly clearer ("crash" is kind of strange to read in Drupal code).

+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -320,9 +320,9 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
+        // automatically by the Renderer. This prevents a definite crash, when
+        // #lazy_builder callback is invoked.

s/This prevents … invoked./Pretending they are supported allows us to avoid throwing an exception 100% of the time./

#2572613: Fix 'Drupal.Array.Array.CommaLastItem' coding standard is the existing standards issue for trailing commas, so updated the split-off issue to that instead. Thanks!

Edits from #17 on patch at #16

*2853684-19.patch
and
*interdiff-16-19.txt

+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -321,8 +321,8 @@
+        // automatically by the Renderer.Pretending they are supported allows

Missing space after period.

Space added on #19

Thanks!

Ah, I get it now, thanks!

The word "pretending" was still a little weird for the tone, so I made a very small rewording on commit:

diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php
index c88e054433..503359ef78 100644
--- a/core/lib/Drupal/Core/Render/Renderer.php
+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -321,8 +321,8 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
         '#cache',
         '#create_placeholder',
         // The keys below are not actually supported, but these are added
-        // automatically by the Renderer. Pretending they are supported allows
-        // us to avoid throwing an exception 100% of the time.
+        // automatically by the Renderer. Adding them as though they are
+        // supported allows us to avoid throwing an exception 100% of the time.
         '#weight',
         '#printed'
       ];

Committed to 8.4.x and cherry-picked to 8.3.x. Thanks!

Sorry for my confusing language :(