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

feat: Add statement_indentation config not_for_comments #7963

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rudiedirkx
Copy link

@rudiedirkx rudiedirkx commented Apr 22, 2024

New option to completely ignore // commented lines. I like my temporary comments to be very explicit, not neatly indented with the rest of the code. Real comments, I'll correctly indent myself, these temp comments, I'll put at the start of the line, or maybe more indented than the rest, whatever stands out.

E.g., while playing with phpstan:

 public function buildForm(): void {
 // \PHPStan\dumpType($this->model); // << Leave this one alone
-$this->platform = $this->requireData('platform'); // << Indent this one 1 level
+	$this->platform = $this->requireData('platform'); // << Indent this one 1 level
 	$this->required = $this->requireData('required');
 
 	$this->platform->buildKeyForm($this, $this->required);
}

@coveralls
Copy link

coveralls commented Apr 22, 2024

Coverage Status

coverage: 96.032% (+0.002%) from 96.03%
when pulling 081423b on rudiedirkx:statement_indentation-not_for_comments
into 37d8d45 on PHP-CS-Fixer:master.

@rudiedirkx rudiedirkx changed the title statement_indentation config not_for_comments feat: Add statement_indentation config not_for_comments Apr 22, 2024
@mvorisek
Copy link
Contributor

I would prefer option to "allow_zero_indented_comments" to keep comments with zero indentation, but align every other comments with non-zero indentation.

@rudiedirkx
Copy link
Author

I agree. And that's also how I use it, but not how I made it. I'll give that a shot. I'm not sure I understand the $tokens magic well enough (white-space tokens seem to cross EOL..?), but I'll give it a go.

@Wirone Wirone marked this pull request as draft April 23, 2024 08:18
@@ -7,6 +7,15 @@ Each statement must be indented.
Configuration
-------------

``not_for_comments``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
``not_for_comments``
``skip_single_line_comments``

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like allow_zero_indented_comments better, because that's what it does. I have the occasional two lines of // \PHPStan\dumpType() at line start.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. Make sure it is only for single line comments starting with //.

@rudiedirkx

This comment was marked as resolved.

@rudiedirkx

This comment was marked as resolved.

@julienfalque
Copy link
Member

This looks like a very specific edge case, I'm not sure we want to support it natively. Is this part of some widespread code style?

@rudiedirkx
Copy link
Author

No widespread code style as far as I know.

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

Successfully merging this pull request may close these issues.

None yet

4 participants