PHP-CS-Fixer icon indicating copy to clipboard operation
PHP-CS-Fixer copied to clipboard

PHPDocAlign - Make alignment of multi-line descriptions configurable

Open dontub opened this issue 8 years ago • 13 comments

This change allows to configure the alignment of multi-line descriptions in the phpdoc_align fixer. Especially useful for @param annotations if type hint and variable name are relatively long to avoid very long lines or lots of description lines.

dontub avatar Dec 05 '17 12:12 dontub

Hi and thanks for sending this PR! I've a few questions on concept level and there is a SCA thingy that the CI tripped on (we don't allow switch constructions in the code base). About the new options, will these with their defaults behave the same as the fixer currently does?

SpacePossum avatar Dec 06 '17 12:12 SpacePossum

also, is there some well-established standard / community that would benefit from that option ?

keradus avatar Dec 06 '17 12:12 keradus

With the default options the fixer behaves the same as the fixer currently does.

also, is there some well-established standard / community that would benefit from that option ?

At least indentation by 1, 2, or 4 spaces I've found in different style guides. though there is no alignment of names or type hints: https://www.drupal.org/docs/develop/coding-standards/api-documentation-and-comment-standards https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Documentation https://google.github.io/styleguide/javascriptguide.xml?showone=Comments#Comments https://google.github.io/styleguide/javaguide.html#s7-javadoc

At least here it is suggested to align everything and add add an extra space: https://en.wikibooks.org/wiki/PHP_Programming/Coding_Standards#Sample_File

I'll replace the switch if you agree in general with this PR.

dontub avatar Dec 06 '17 17:12 dontub

fact that there are indeed established standards for using 2 spaces for typescript and so has nothing to do with PHP. but ok, even in 3 PHP links you provided there are different way.

:+1: for idea in that case

keradus avatar Dec 06 '17 22:12 keradus

The switch construct is replaced.

The first links where just to show that there exists style guides with only space indent on multi-line (description_align = tag, description_extra_indent = n). This would combine alignment with indentation of n spaces on multi-line comments, so yes it's not exactly the same as in those guides.

The final example could be achieved (apart from the double space after the type hint) with description_extra_indent = 1.

dontub avatar Dec 07 '17 12:12 dontub

Is there something keeping you off from merging?

dontub avatar Jan 25 '18 12:01 dontub

hi @dontub and sorry for the wait, we are trying to catch up with all the PR's and issues, hope to review this one soon

SpacePossum avatar Jan 25 '18 12:01 SpacePossum

Thanks for the feedback. I've found some time today to look at it again. I'm wondering how this example should be aligned by default:

/**
 * @param int $foobar Desc
 * ription
 * @return int Desc
 * ription
 */

This is what my code does at the moment:

/**
 * @param  int $foobar Desc
 *                     ription
 * @return int Desc
 *                     ription
 */

This is what PHP-CS-Fixer does at the moment (one space less in the last description line):

/**
 * @param  int $foobar Desc
 *                     ription
 * @return int Desc
 *                    ription
 */

This is what I would expect:

/**
 * @param  int $foobar Desc
 *                     ription
 * @return int         Desc
 *                     ription
 */

Shall I change this PR regarding to this issue?

dontub avatar May 12 '18 12:05 dontub

@julienfalque have you seen my questions in the comments above?

dontub avatar Jun 11 '18 18:06 dontub

@SpacePossum @julienfalque I'd like to see this merged. Though I'm wondering if I should take the time to resolve the conflicts again because there was no response since the last changes...

dontub avatar Feb 26 '19 15:02 dontub

@dontub please, could you reupdate and solve the conflicts? Could be a chance to be merged in future releases and maintainer will probably comment about your last suggestion. Thanks for your effort

shakaran avatar Sep 04 '21 14:09 shakaran

@shakaran: I would resolve conflicts once again, if a maintainer shows interest in merging this. Otherwise I think it's wasted time.

dontub avatar Sep 11 '21 19:09 dontub

Since this pull request has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 14 days.

Please keep your branch up-to-date by rebasing it when main branch is ahead of it, thanks in advance!

github-actions[bot] avatar Feb 24 '24 12:02 github-actions[bot]