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

Generate only 1, not 2 newlines at the end of baseline #1243

Merged
merged 6 commits into from Apr 29, 2022

Conversation

VasekPurchart
Copy link
Contributor

@VasekPurchart VasekPurchart commented Apr 25, 2022

We have noticed, that baseline has two newlines at the end of file, which is weird because the convention for most text based files is to have one. I believe PHPStan follows it as well, even for neon, e.g.: https://github.com/phpstan/phpstan-src/blob/831469c9c406f18e537dd8c46b8b9b28a3897450/build/phpstan.neon .

But baseline is generated using Neon::encode() and this puts two newlines at the end of file. From my research I believe this is due to the fact that the encoding puts empty lines between array items and there is no exception for the last one.

I do not believe that changing this in Neon is desirable - too big of a change for too little gain. Plus if it were to be omitted only after the last item, it could lead to uneven results with concatenating multiple separately encoded neons etc. On the other hand in PHPStan we already know the structure of the whole file, so we can solve it here easily.

I included measures not to change the baseline file if this would be the only change in order to prevent bugging people with it. It will disappear with the next "real" change.

@VasekPurchart VasekPurchart changed the base branch from 1.6.x to 1.5.x April 25, 2022 15:37
@VasekPurchart VasekPurchart changed the base branch from 1.5.x to 1.6.x April 26, 2022 07:15
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

I took this step so that this change can be properly testable: 3a3c69e

Wer're now free to change the formatErrors() method signature to pass in an existing baseline file.

The desired behaviour can now be nicely tested in BaselineNeonErrorFormatterTest.

@VasekPurchart VasekPurchart marked this pull request as ready for review April 28, 2022 15:53
@ondrejmirtes ondrejmirtes merged commit 153e051 into phpstan:1.6.x Apr 29, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@VasekPurchart VasekPurchart deleted the baseline-duplicate-eol branch April 29, 2022 09:41
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