Problem/Motivation

After #1876018: Displaced space in core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php has been committed I stumble upon lots of "disabled spaces". Both ways are possible but I think that we have to agree on a standard for that.

$ grep -irn " =&" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme | wc -l
50
$ grep -irn " = &" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme | wc -l
328

The results include some regex expressions but you can see that the = & way is more present than the other way. After the code freeze we can start with writing patches for that. Note that this part is missing in the coding standards, so we also have to update them.

Benefits

If we adopted this change, the Drupal Project would benefit by ...

Three supporters required

  1. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
  2. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
  3. https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)

Proposed changes

Provide all proposed changes to the Drupal Coding standards. Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:

1. Operators

Current text

All binary operators (operators that come between two values), such as +, -, =, !=, ==, >, etc. should have a space before and after the operator, for readability. For example, an assignment should be formatted as $foo = $bar; rather than $foo=$bar;. Unary operators (operators that operate on only one value), such as ++, should not have a space between the operator and the variable or number they are operating on.

Proposed text

All binary operators (operators that come between two values), such as +, -, =, !=, ==, >, etc. should have a space before and after the operator, for readability. For example, an assignment should be formatted as $foo = $bar; and an assignment by reference as $foo = &$bar.
Unary operators (operators that operate on only one value), such as ++, should not have a space between the operator and the variable or number they are operating on.

2. Repeat the above for each page or sub-page that needs to be changed.

Remaining tasks

  1. Create this issue in the Coding Standards queue, using the defined template
  2. Add supporters
  3. Create a Change Record
  4. Review by the Coding Standards Committee
  5. Coding Standards Committee takes action as required
  6. Discussed by the Core Committer Committee, if it impacts Drupal Core
  7. Final review by Coding Standards Committee
  8. Documentation updates
    1. Edit all pages
    2. Publish change record
    3. Remove 'Needs documentation edits' tag
  9. If applicable, create follow-up issues for PHPCS rules/sniffs changes

For a full explanation of these steps see the Coding Standards project page

Comments

yannickoo’s picture

yannickoo
Location Berlin
commented
Issue summary: View changes

Typo

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: Agree on a standard for assigning references to variables » [policy] Agree on a standard for assigning references to variables
Issue tags: +Coding standards

Tagging and standardizing issue title for a standards issue.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

Splitted block.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Project: Drupal core » Drupal Technical Working Group
Version: 8.0.x-dev »
Component: documentation » Documentation
Issue summary: View changes

Coding standards decisions are now supposed to be made by the TWG

tizzo’s picture

tizzo commented
Project: Drupal Technical Working Group » Coding Standards

Moving this issue to the Coding Standards queue per the new workflow defined in #2428153: Create and document a process for updating coding standards.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

In a directory containing Drupal 11 files, grep -irn "=&" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme| wc -l returned 93, while grep -irn "=&" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme| wc -l returned 328.

Truly, the first command also count lines like the following ones.

./core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php:1357:          'To maintain the capabilities of this text for
mat, <a target="_blank" href="/admin/help/ckeditor5#migration-settings">the CKEditor 5 migration</a> did the following: Enabled thes
e plugins: (<em class="placeholder">Link, Block quote, Code, List</em>). Added these tags/attributes to the Source Editing Plugin\'s
 <a target="_blank" href="/admin/help/ckeditor5#source-editing">Manually editable HTML tags</a> setting: &lt;cite&gt; &lt;dl&gt; &lt;dt&gt; &lt;dd&gt; &lt;a hreflang&gt; &lt;blockquote cite&gt; &lt;ul type&gt; &lt;ol type=&quot;1 A I&quot;&gt; &lt;h2 id=&quot;jump-*&quot;&gt; &lt;h3 id&gt; &lt;h4 id&gt; &lt;h5 id&gt; &lt;h6 id&gt;. Additional details are available in your logs.',
./core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php:1401:          'As part of migrating to CKEditor 5, it was found that the <em class="placeholder">A CKEditor 4 configured to have span styles</em> text format\'s HTML filters includes plugins that support the following tags, but not some of their attributes. To ensure these attributes remain supported, the following were added to the Source Editing plugin\'s <em>Manually editable HTML tags</em>: &lt;span class=&quot;llama&quot;&gt;. The text format must be saved to make these changes active.',
./core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php:1406:          'To maintain the capabilities of this text format, <a target="_blank" href="/admin/help/ckeditor5#migration-settings">the CKEditor 5 migration</a> did the following:  Added these tags/attributes to the Source Editing Plugin\'s <a target="_blank" href="/admin/help/ckeditor5#source-editing">Manually editable HTML tags</a> setting: &lt;span class=&quot;llama&quot;&gt;. Additional details are available in your logs.',
./core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php:553:          'The following tag is not valid HTML: <em class="placeholder">&lt;blockquote class=&quot;&quot;&gt;</em>.',

Probably, the following commands give a more precise result.

  • grep -irn " =&" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme | wc -l (50)
  • grep -irn " = &" ./ --include=*.module --include=*.php --include=*.inc --include=*.install --include=*.theme | wc -l (328)
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

(I apologize: I added a sentence to the issue summary instead of putting it on my comment.)

quietone’s picture

quietone commented
Issue summary: View changes

This was discussed at coding standards meetings, #3456119: Coding Standards Meeting Tuesday 2024-06-18 2100 UTC.

The existing text does cover this case but there was a question if the text should make clear that 'operator' does not mean any combination of operators.

And a sniff will be needed to ensure the agreed format is enforced.

quietone’s picture

quietone commented
Title: [policy] Agree on a standard for assigning references to variables » Agree on a standard for assigning references to variables
Component: Documentation » Coding Standards
catch’s picture

catch
he/him
Primary language English
commented

Yeah I think it's implicit in the existing coding standard but we should make it explicit.

quietone’s picture

quietone commented
Title: Agree on a standard for assigning references to variables » Standard for assigning references to variables
Issue summary: View changes
Status: Active » Needs review

Just for info:

In addition to the basic assignment operator, there are "combined operators" for all of the binary arithmetic, array union and string operators that allow you to use a value in an expression and then set its value to the result of that expression. For example:

and

Assignment by reference is also supported, using the "$var = &$othervar;" syntax. Assignment by reference means that both variables end up pointing at the same data, and nothing is copied anywhere.

I updated the issue summary with a proposed text. It add an example, removes the two related 'instead of' examples and moved the 'unary to a separate paragraph. I removed the 'instead of' because I find pointing out what not to do isn't helpful.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

In the following sentence, I would use a different operator as example.

Unary operators (operators that operate on only one value), such as ++, should not have a space between the operator and the variable or number they are operating on.

$var = 1++; contains a syntax error, like echo 1++, "\n";.

I am not sure how to make that sentence better, considering that even new is a unary operator, for which there must be a space between the operator and its operand. (their operand can replace the variable or number they are operating on, but that role is not valid for all the unary operators.)