Problem/Motivation

Fields with exceptionally long names throw a coder warning when exported.

Coding standards state:

Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.
Control structure conditions may exceed 80 chars, if they are simple to read and understand:

Steps to reproduce:
1. Create a field with a very long exported name. This is especially easy to do when the field in a field collection. Example: // 'field_collection_item-field_alpha_channel_sponsored-field_alpha_sponsor_bundle_title'
2. Export above field using features.
3. Run phpcs with Drupal standards.
4. Coder returns: Line exceeds 80 characters; contains 92 characters.

Proposed resolution

Code sniffer should not throw a warning if the comment string contains no spaces.

CommentFileSizeAuthor
#3 wrap-long-field-names-2466961-3.patch2.39 KBmarkdorison
#1 wrap-long-field-names-2466961-1.patch2.43 KBmarkdorison
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
FileSize
wrap-long-field-names-2466961-1.patch2.43 KB

The attached patch solves the issue when wrapping the identifier under the prefix will result in two comment lines both under 80 characters: 

// Exported field_instance:
// 'field_collection_item-field_project_process-field_project_process_text'

I don't have a proposed solution if the identifier itself is over 80 characters (which I have seen) but this is an improvement that probably covers a large number of cases.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Active » Needs review
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
FileSize
wrap-long-field-names-2466961-3.patch2.39 KB

Small style change.

tflanagan’s picture

tflanagan CreditAttribution: tflanagan commented

This looks good and is very much needed. I have had to manually do what your code does when codesniffer doesn't like something!

mpotter’s picture

mpotter CreditAttribution: mpotter commented
Status: Needs review » Reviewed & tested by the community

This seems to work well for me.

bedlam’s picture

bedlam
Location Lyon
CreditAttribution: bedlam commented

Works for me as well. Would be nice to eliminate this particular source of PHPCS warnings.

mpotter’s picture

mpotter CreditAttribution: mpotter commented
Status: Reviewed & tested by the community » Fixed

Committed to 598d4e5.

  • mpotter committed 598d4e5 on 7.x-2.x authored by markdorison 
    Issue #2466961 by markdorison: Long field names trigger "Line exceeds 80...
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Issue summary: View changes
Status: Fixed » Active

Super pleased to see this committed. Since the commit was an improvement and not a complete fix of the problem, I am going to reopen the issue so we can continue to track (and hopefully fully resolve) this issue.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@markdorison what needs to be further done here?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@joelpittet I updated the summary; the short answer is I am not sure. How do we pass Drupal code standards of comments no longer than 80 characters long when we are outputting a comment of a field's name that is longer than 80 characters? Some (maybe not great) ideas:

1. Chop the field name in the middle at an underscore and wrap it to two lines.
2. Truncate the field name.
3. Not include the comment line if it is longer than 80 characters.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@markdorison We have a bit of leniency it seems when variables and URLs escape the bounds of this rule, though I'm not sure how the automated tools like coder or CodeSniffer deal with this?

So personally I'd let it run;)

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented

@joelpittet The problem is that the automated tools flag this since it is technically not following Drupal code standards. Anyone using Features who is also automating those tests is going to have a hard time getting them to pass cleanly which is frustrating.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Maybe those concessions can be added to the automated tools?

Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.
Control structure conditions may exceed 80 chars, if they are simple to read and understand:

https://www.drupal.org/coding-standards#linelength

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Project: Features » Coder
Component: Code » Coder Sniffer
Issue summary: View changes

@joelpittet Thanks for pointing out that exception. At this point, I am moving the issue back to the Coder issue queue and updating the summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Version: 7.x-2.x-dev » 8.x-2.x-dev
Status: Active » Postponed (maintainer needs more info)

Could you post an example code snippet to the issue summary that demonstrates the false positive in Coder? And this should be fixed in 8.x-2.x first.

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented

There is a bug in the regex that checks the "Contains" line, it should also match underscore characters.

  preg_match('/^Contains [a-zA-Z_\\\\.]+$/', $tokens[($stackPtr - 2)]['content']) === 1)
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented

@benjy: that does not seem to be related to this issue, could you file a new issue? There should be an issue somewhere that deals with the "Contains ..." line.

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented

@klausi, ah crap, i had both issues open and commented on the wrong one: https://www.drupal.org/node/2530920#comment-10516578

That's where it went in, not sure if you want a follow-up?

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active

@klausi: Updated summary with a new field name export example that fails the current tests:

  // Exported field_instance:
  // 'field_collection_item-field_alpha_channel_sponsored-field_alpha_sponsor_bundle_title'
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Active » Fixed

This has been fixed in Coder with some other issue. If the comment line is only one word then Coder will not complain if it exceeds 80 characters.

Please test!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

sureshravi’s picture

sureshravi CreditAttribution: sureshravi commented

Hi, guys am doing a phpcs drupal standard for my Drupal project. whenever i hit the cmd in terminal phpcs --standard=Drupal .\test.php

it's throwing a warning Line exceeds 80 characters; contains 86 characters. how can I resolve this? Without changing line exceeds the rule(phpcs XML config rule)