Open mfrieling opened 10 months ago
@mfrieling
I would like to discuss extending this enhancement to the first line of a multi-line comment, also known as the short description. The short description is 96 characters long (excluding the
* ) and would be broken up into two lines. But that would cause the sniff Drupal.Commenting.DocComment.ShortSingleLine to trigger an error.
The max line length is configurable for this sniff. Making an exception for the short description is not something I would accept a PR for.
The fact that Drupal enforces the short description to be single line is not something this sniff should take into account. That's something to discuss in the Drupal project.
Also keep in mind that the short description is supposed to be exactly that : short. You can expand on it in a long description.
For your example, that could look like this:
/**
* Provides a configurable slideshow block.
*
* This slideshow block is used on static pages if they are top-level in the menu.
*
* @Block(
* ...
* )
*/
And in the case of the admin_label line, if the text provided to @Translation() should be longer that would exceed the maximum line length as well.
Aside from the max line length being configurable, you can also choose to ignore comments altogether.
Only just realized this issue was opened in the Squizlabs repo. For the future, please open new issues in the https://github.com/PHPCSStandards/PHP_CodeSniffer repo.
See #3932 for context.
@jrfnl thanks for the feedback. Actually it is more complicated than I thought and the reason why I didn't find the Generic.Files.LineLength
sniff when searching for my issue. The Drupal rulesets don't reference this one. I checked my project and actually never got a PHPCS error regarding the line length.
Besides that, PhpStorm actually seems to use a combination of Editorconfig, PHPCS rules and it's own config when reformatting code on file save. That makes it more difficult to find out why a piece of code gets formatted the way it does and how to configure different tools to work nicely together (I had similar problems with Jetbrains IDEs and ESLint + Prettier). Because the comment line only got broken up when PhpStorm was configured to use "PHP Code Beautifier and Fixer" as external formatter with using the project phpcs.xml.dist
file, I thought it must be coming from therre. But I just found out that the line length of 80 seems to be only configured in PhpStorms own settings.
Back to your suggestions:
By ignoring the comments via Generic.Files.LineLength
altogether I got max line length exceeded errors to start showing up (this shows that Drupal doesn't utilize this sniff). Because that produces tons of new errors it is not a option for the moment. I will have to fix that at a later point as my main goal for now is upgrading to PHP 8.2 and latest Drupal version.
But in general ignoring comments line length of course would be an option. A better one would be to be able to set different line length limits for comments than for code.
Rewriting the short description in this case might be possible, even if I would loose important information (for "static pages if they are top-level in the menu") from IntelliSense help. In other cases where super long names are involved it is more difficult.
To sum it up
This is a follow-up to #766 where the reason for too long lines in comments was basically the line containing an url.
I would like to discuss extending this enhancement to the first line of a multi-line comment, also known as the short description. Assume the following example of a class in Drupal 10:
The short description is 96 characters long (excluding the
<space>*<space>
) and would be broken up into two lines. But that would cause the sniffDrupal.Commenting.DocComment.ShortSingleLine
(Doc comment short description must be on a single line, further text should be a separate paragraph) to trigger an error. In some cases it is just not possible to make the short description of a class or function fitting the maximum line length, if special naming or wording of things (coming from the technology used like Drupal, from the customer's/project's business domain or similar).And in the case of the _adminlabel line, if the text provided to
@Translation()
should be longer that would exceed the maximum line length as well. As @PortNumber53 mentioned in a comment to #766 breaking the lines inside of annotations does not always work well.