Minor update whitespace PHPCS rules#6666
Conversation
To simplify our configuration, apply more rules, and be clearer about what is added or removed compared with PSR12. Does not change our current conventions, but just a bit more consistent.
| switch($type_get) { | ||
| case 'c': |
There was a problem hiding this comment.
Most modified lines are due to wrongly indented switch cases.
|
@math-GH FYI, in VS Code, the extension https://marketplace.visualstudio.com/items?itemName=ValeryanM.vscode-phpsab will offer live feedback about those PHPCS rules, including whitespace:
|
|
Any comment @Frenzie ? |
| case 'M': case 'm': return (int)$size_str; | ||
| case 'K': case 'k': return (int)$size_str / 1024; | ||
| case 'G': case 'g': return (int)$size_str * 1024; | ||
| case 'M': |
There was a problem hiding this comment.
I personally prefer indented case inside switch statements. But when I set up the rules I tried to make them as close to the existing style as possible, not to add in my own opinion. ;-)
There was a problem hiding this comment.
Yes, me too, and it is also more common to have indented cases in switches, I believe
| private function getFilenamePrefix(string $key): string { | ||
| return preg_replace('/\..*/', '.php', $key) ?? ''; | ||
| } | ||
|
|
There was a problem hiding this comment.
This strikes me as a potentially somewhat unfortunate thing to enforce.
There was a problem hiding this comment.
Ok, I will exclude a few more rules
p/api/fever.php
Outdated
| * See https://github.com/dasmurphy/tinytinyrss-fever-plugin | ||
| */ | ||
|
|
||
| declare(strict_types=1); |
There was a problem hiding this comment.
Meh, reduces clarity by hiding it further down.
There was a problem hiding this comment.
Rule about headers order disabled
app/Controllers/indexController.php
Outdated
| as $entry) { | ||
| $type, $id, FreshRSS_Context::$state, FreshRSS_Context::$order, | ||
| $postsPerPage ?? FreshRSS_Context::$number, FreshRSS_Context::$offset, FreshRSS_Context::$first_id, | ||
| FreshRSS_Context::$search, $date_min) as $entry) { |
There was a problem hiding this comment.
Disabling Squiz.ControlStructures.ForEachLoopDeclaration.SpacingBeforeAs completely would prevent checking spaces in foreach ($list as $item) {}, and since the problem occurs only once in the whole codebase, I came with another workaround
There was a problem hiding this comment.
Oh, I didn't mean to insist too strongly. :-)
* Align CI rules with main FreshRSS repository Including: * FreshRSS/FreshRSS#6666 * FreshRSS/FreshRSS#6668 * Sync FreshRSS
Regression from FreshRSS#6666 We were not checking anymore for whitespace rules in e.g. `if (true) {`
Regression from #6666 We were not checking anymore for whitespace rules in e.g. `if (true) {`
To simplify our PHPCS configuration, apply more rules, and be clearer about what is added or removed compared with PSR12.
Does not change our current conventions, but just a bit more consistent.
Useful commands:
make test-all make fix-all # or individually: composer run-script phpcs composer run-script phpcbf