Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow lines to be longer than 80 chars if they're the first line in multi-line comments or part of annotations #3938

Open
mfrieling opened this issue Jan 22, 2024 · 3 comments

Comments

@mfrieling
Copy link

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:

/**
 * Provides a configurable slideshow block, used on static pages if they are top-level in the menu.
 *
 * @Block(
 *   id = "slideshow_block",
 *   admin_label = @Translation("Slideshow block on static pages."
 * )
 */
class CustomSlideshowBlock extends BlockBase {
}

The short description is 96 characters long (excluding the <space>*<space>) and would be broken up into two lines. But that would cause the sniff Drupal.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 admin_label 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.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 22, 2024

@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.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 22, 2024

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.

@mfrieling
Copy link
Author

@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

  • I will work around this issue by adjusting the project's PhpStorm configuration, rulesets etc. to achieve the needed results.
  • I recommend improving the overall documentation (as far as this concerns PHPCS, but also contributors of other coding standard definitions and rulesets) in a way that people can find what they need without knowing the actual name of the sniff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants