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

[Core] Remove no longer needed empty start and end tag check on BetterStandardPrinter #1559

Merged
merged 1 commit into from
Dec 24, 2021

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Dec 24, 2021

@staabm as there is recursive check on resolveNewStmts() on

return $this->resolveNewStmts($stmts[0]->stmts);

The empty start tag and end tag seems no longer needed, and I tried e2e tests seems working ok.

Screen Shot 2021-12-24 at 17 43 56

ref #1329

@samsonasik samsonasik changed the title [Core] Remove no longer needed empty start and end tag on BetterStandardPrinter [Core] Remove no longer needed empty start and end tag check on BetterStandardPrinter Dec 24, 2021
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@TomasVotruba
Copy link
Member

Thanks. I'm not sure if this was Windows only issue, the we currently do not test.

@staabm Could you confirm this will not break your Rector run?

@samsonasik
Copy link
Member Author

@TomasVotruba the recursive method call is from PR https://github.com/rectorphp/rector-src/pull/1329/files#diff-35b384f5686672e9b726b991a0c45dd8c18cfc8b365953489bf95a39db82205fR448 which it already fix it, without the recursive call, it applied the start and end:

Screen Shot 2021-12-24 at 17 36 53

With recursive check, it already solve it:

Screen Shot 2021-12-24 at 17 37 43

@samsonasik
Copy link
Member Author

samsonasik commented Dec 24, 2021

1. Without recursive method call, running e2e from plain-views directory got invalid as expected (add start open tag at first and end)

Screen Shot 2021-12-24 at 17 40 13

2 With Recursive call, it shows valid as expected (no start open tag at first and end)

Screen Shot 2021-12-24 at 17 43 56

@TomasVotruba
Copy link
Member

I see. Thanks for verification and examples 👍

@TomasVotruba TomasVotruba merged commit 5157b7e into main Dec 24, 2021
@TomasVotruba TomasVotruba deleted the remove-no-longer-need-check-empty-start-end-tag branch December 24, 2021 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants