Problem/Motivation
State what you believe is wrong or missing from the current standards.
Benefits
If we adopted this change, the Drupal Project would benefit by ...
Three supporters required
- https://www.drupal.org/u/{userid} (date that user added support)
- https://www.drupal.org/u/{userid} (date that user added support)
- https://www.drupal.org/u/{userid} (date that user 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. {link to the documentation heading that is to change}
Current text
Add current text in blockquotes
Proposed text
Add proposed text in blockquotes
2. Repeat the above for each page or sub-page that needs to be changed.
Remaining tasks
Create this issue in the Coding Standards queue, using the defined template- Add supporters
- Create a Change Record
- Review by the Coding Standards Committee
- Coding Standards Committee takes action as required
- Tagged with 'Needs documentation edits' if Core is not affected
- Discussed by the Core Committer Committee, if it impacts Drupal Core
- Documentation updates
- Edit all pages
- Publish change record
- Remove 'Needs documentation edits' tag
- If applicable, create follow-up issues for PHPCS rules/sniffs changes
For a fuller explanation of these steps see the Coding Standards project page
==== Original template to be converted to the new template above and then removed ===
Problem/Motivation
Note that if the line declaring an array spans longer than 80 characters (often the case with form and menu declarations), each element should be broken into its own line, and indented one level:
Coder currently also has an exception for arrays of only two items - only arrays with three or more items receive a warning for being over 80 characters.
However, this is impractical in certain circumstances:
1. In a complex form, I'm altering #parents to remove wrapping elements from the structure of submitted values in form state. The array goes over 80 characters on the line, but breaking it into individual lines is awkward for this type of simple definition
class {
function () {
foreach(...) {
foreach (...) {
$form[$policyTypeKey]['directives'][$directiveName]['options']['flags_wrapper']['flags'] = [
'#type' => 'checkboxes',
'#parents' => [$policyTypeKey, 'directives', $directiveName, 'flags'],
'#options' => [
'unsafe-inline' => "'unsafe-inline'",
'unsafe-eval' => "'unsafe-eval'",
'unsafe-hashes' => "'unsafe-hashes'",
'unsafe-allow-redirects' => "'unsafe-allow-redirects'",
'strict-dynamic' => "'strict-dynamic'",
'report-sample' => "'report-sample'",
],
'#default_value' => $config->get($policyTypeKey . '.directives.' . $directiveName . '.flags') ?: [],
];
}
}
}
}
2. Later when retrieving the form values during validation or submission, they are accessed by specifying the keys in an array which will also be longer than 80 characters.
class {
function () {
foreach (...) {
$reportHandlerOptions = $form_state->getValue([$policyTypeKey, 'reporting', $reportHandlerPluginId]);
}
}
}
Proposed resolution
Update the note in the standard to:
Note that if the line declaring an associative array contains three or more elements and spans longer than 80 characters (often the case with form and menu declarations), each element should be broken into its own line, and indented one level
e.g.
This would not produce a warning:
$variable = ['a', 'really', 'long', 'array', 'that', 'is', 'longer', 'than', 80, 'characters];
Comments
Comment #2
liam morlandIt should be clear that the limit only applies to lines that contain only an array declaration. So, for example, the following would be allowed, despite being over 80 characters:
Comment #3
gappleCoder would not produce a warning on that example, because the array only contains a single item. The following would currently produce a warning since it contains three items:
----
There are currently no notes or recommendations regarding function calls, but for control structure conditions the standard recommends to split out and prepare the conditions separately, which would correspond to:
----
Alternately in such instances, I would treat long function calls the same as long arrays, breaking parameters individually to separate lines
or
Comment #4
gappleI don't see a distinction between assigned arrays and directly passed arrays and think they should be treated equally, breaking up long and nested arrays where necessary.
Comment #5
pfrenssenI would prefer this line length limitation to be entirely dropped, and this section be replaced with something that is at the developer's discretion.
I also want to point out that this standard has not been included in the core phpcs ruleset so it has never been enforced. Core is rife with array definitions that exceed 80 characters. So this is a standard that would be rather non-controversial to drop.
Current coding standard
Proposed coding standard
I think the above code sample is a perfect example. It comes from
\Drupal\locale\Form\TranslateEditForm::buildForm(). It is compact and has just the right amount of line splitting to make it super easy to read. With our current coding standards rule it would devolve into this monstrosity:Comment #6
pfrenssenComment #7
alexpott+1 to #5 I don't think it's possible to have a good coding standard specifically for arrays. If we look at PSR standards there are reasonable line length standards that we could look to. But singling out arrays doesn't make much sense.
Comment #8
dwwAgreed we should relax this standard.
Also agreed that we should reconsider 80 char limit entirely, but that’s for #3173782: Increase line length limit to 120, not here.
Comment #9
catchMakes sense to me to make this apply to associative arrays only, or we could possibly relax it further than that.
Comment #10
pfrenssenIn the latest Coder release we increased the threshold for the current array splitting sniff from 80 to 120 characters, but the frustration remains even though it has become much less frequent.
Comment #11
quietone commentedThe Coding Standards Committee has developed a custom issue template to assist in managing issues efficiently. We have already found that it's use is helping. For issues created before the template was in use I am adding the new template to the Issue Summary and asking everyone here to help convert to it..
Thank you for your help!
Comment #12
quietone commentedAs per #10, array can span more that 80 characters. This also needs an MR with proposed changes.