Uh oh!
There was an error while loading. Please reload this page.
Tidy infrastructure: Reindent PHP/PHPT - #5074
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). |
nikic
commented
Jan 10, 2020
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 |
nikic
commented
Jan 10, 2020
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. |
kocsismate
commented
Jan 10, 2020
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... |
kocsismate
commented
Feb 2, 2020
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. |
nikic
commented
Feb 3, 2020
@kocsismate Done |
kocsismate
commented
Feb 3, 2020
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). 🤔 |
nikic
commented
Feb 3, 2020
@kocsismate This currently targets the 7.4 branch. Could of course only do it for master, but might cause merge issues? |
nikic
commented
Feb 3, 2020
It might make sense to apply the |
kocsismate
commented
Feb 3, 2020
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. |
nikic
commented
Feb 3, 2020
I've opened #5145 for a more reduced 7.4 variant. |
nikic
commented
Feb 3, 2020
Now targeting master. |
nikic
commented
Feb 3, 2020
Travis failures: |
kocsismate
commented
Feb 3, 2020
@nikic Yeah, I saw. I've been fixing them :) |
kocsismate
commented
Feb 3, 2020
... and done in 0253a23 |
nikic
commented
Feb 3, 2020
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 |
kocsismate
commented
Feb 3, 2020
Noted. Fix is on the way. |
kocsismate
commented
Feb 3, 2020
9942f45 hopefully fixes the issues... (this time I restricted myself not to change the indentation of format specifiers in order to reduce the scope). |
nikic
commented
Feb 3, 2020
nikic
commented
Feb 3, 2020
@kocsismate Thanks a lot for sorting out all the formatting issues! |
You're welcome :) |
Add
scripts/dev/tidy.phpto 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.