-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Tidy infrastructure: Reindent PHP/PHPT #5074
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
Conversation
Yes! I was also thinking about something like this for only .phpt files. Indentation was my frustration too (space vs tabs + 2 vs 4 spaces), however I could also imagine having PSR-12 formatting where it is applicable (ok, it's wishful thinking at this point). |
I've decided to avoid the C reindentation for now (as the results are somewhat mixed -- because tabs). Instead this now implements the actually important part, which is PHP and PHPT reindentation. There's a new |
Quite a few failures due to tests that have trailing whitespace that is also part of the output :( Those will all have to be fixed first. |
Can't they all be just marked |
I think it would be better to avoid that marker unless the test really tests something about whitespace (e.g. flexible heredoc parsing). Fixing is more along the lines of e748a5a... |
I fixed as many indentation/trailing whitespace issues as I could in 2015c7a, so can you please rebase to see how many failing tests are left? I can deal with them as well. |
@kocsismate Done |
Is it possible that it's not the latest master? I found some failures where the expected output is the same as in current master (e.g. ext/dom/tests/DOMNode_cloneNode_basic.phpt). 🤔 |
@kocsismate This currently targets the 7.4 branch. Could of course only do it for master, but might cause merge issues? |
It might make sense to apply the |
Yeah, it sounds reasonable. Although, I already tried out what happens when I cherry-pick the changes I introduced in master to the 7.4 branch. The result doesn't seem very bad, so I am ok to redo the fixes there as well if it's beneficial at the long-term. |
I've opened #5145 for a more reduced 7.4 variant. |
In preparation for phpGH-5074
In preparation for phpGH-5074
Now targeting master. |
Travis failures:
|
@nikic Yeah, I saw. I've been fixing them :) |
... and done in 0253a23 |
There are some 32-bit failures: https://dev.azure.com/phpazuredevops/PHP/_build/results?buildId=5252&view=ms.vss-test-web.build-test-results-tab |
Noted. Fix is on the way. |
9942f45 hopefully fixes the issues... (this time I restricted myself not to change the indentation of format specifiers in order to reduce the scope). |
@kocsismate Thanks a lot for sorting out all the formatting issues! |
You're welcome :) |
Add
scripts/dev/tidy.php
to automatically enforce our formatting / indentation rules.In particular, this removes trailing whitespace and reindents all PHP files and PHPT tests to spaces. Tests can be marked as
--WHITESPACE_SENSITIVE--
if the whitespace is actually relevant for the test.Reindentation is not performed for C yet, as it sometimes has mixed results.