Symplify: hard-coded PHP_EOL

[orklah]

when #1650 will pass, it will remains almost only PHP_EOL issues left on PHPunit.

Do you have a tip to go forward on this?

I need to have a config somewhere (if it doesn't already exists?) that can be accessed everywhere in the code to give me the preferred line feed.

While debugging, I saw a $configuration object. Is it the way to proceed? On what file would you put that configuration?

Thanks

[orklah]

@TomasVotruba any hint how to proceed?

[TomasVotruba]

I'm not sure what PHP_EOL you talk about.

[orklah]

Well, I didn't identified them all, but each instance of PHP_EOL used to build a piece of code will potentially break tests and functionnality on windows. For example:

https://github.com/Symplify/Symplify/blob/e4c3660d7e31c9a1d53de4f2f701b16dfb79bf3d/packages/ChangelogLinker/src/ChangelogDumper.php#L64
This one makes this test fail:
https://travis-ci.org/Symplify/Symplify/jobs/610062568#L341

https://github.com/Symplify/Symplify/blob/e4c3660d7e31c9a1d53de4f2f701b16dfb79bf3d/packages/CodingStandard/packages/TokenRunner/src/Transformer/FixerTransformer/LineLengthTransformer.php#L126
This one makes this test fail:
https://travis-ci.org/Symplify/Symplify/jobs/610062568#L597

This is why I think we need to have a config somewhere to set the type of line feed we want (TBH, it'll probably be \n for a lot of people...)

[TomasVotruba]

I see.

I think you can add EolConfiguration static class with setEolChar() and getEolChar() methods.

[orklah]

Ok, They are in many packages though. Where should I put this class? Is there a place you have configs I can parse or do you think a hard coded value in the class is enough?

[TomasVotruba]

I'd go with most generic one - Symplify\PackageBuilder\Configuration\EolConfiguration
What do you think?

[TomasVotruba]

a hard coded value in the class is enough?

:+1:

[TomasVotruba]

Btw, this might be relevant: https://github.com/sebastianbergmann/phpunit/issues/503

[TomasVotruba]

To be more specific, this method: https://github.com/FriendsOfPHP/PHP-CS-Fixer/commit/d60e963e80f45e2050085964f67c6769f347381f

[orklah]

I'm a bit lost.

I thought the PHP_EOL was causing the error and this was a defect in the diff, but I can't find the issue.

The failed test I took is this one:

6) Symplify\ChangelogLinker\Tests\ChangelogDumper\ChangelogDumperMultipleItemsTest::testReportBothWithPriority
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '\r\n
 ## Unreleased\r\n
-\r\n
 ### SomePackage\r\n
-\r\n
 #### Added\r\n
-\r\n
 Message\r\n
-\r\n
 ### AnotherPackage\r\n
-\r\n
 #### Fixed\r\n
-\r\n
 Another Message\r\n
 \r\n
 '

The expected result is this one it's better in (raw here)

As you can see, there are two line feeds between each line of content. In the original file, they are \n but they are \r\n in my file(github automatically change them?)

The issue is I can't figure out how the code could output two line feeds. I can't explain how it works in the *nix tests.

Can you help?

[TomasVotruba]

This is the file responsible for generating
https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L72

I'd try debug around there

[orklah]

Yeah, I spent an hour on that file on debugging. No matter what I do, I can't explain the two line feeds.

There is always one after each line but I can't explain the second one...

[TomasVotruba]

If you dump the final content of the test into the file instead of console output, what difference is there?

[orklah]

I dumped contents before it even reach the test.

"\r\n## Unreleased\r\n### SomePackage\r\n#### Added\r\nMessage\r\n### AnotherPackage\r\n#### Fixed\r\nAnother Message\r\n\r\n"----
"\r\n## Unreleased\r\n\r\n### SomePackage\r\n\r\n#### Added\r\n\r\nMessage\r\n\r\n### AnotherPackage\r\n\r\n#### Fixed\r\n\r\nAnother Message\r\n\r\n"

First line is returned by the changelogdumper and the second line is the expected file.

The thing is, the return of the changelogdumper is totally coherent to me. It outputs exactly what I would be expecting by reading(and debugging!) the code. What I don't explain is why there is 2 line feeds in the expected file and how the tests can pass on *nix

[TomasVotruba]

Ok. Now we know it's changelog dumper

Which of its methods is adding just 1 newline instead of 2?

[orklah]

https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L95
https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L106
https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L117

There is one PHP_EOL after each line with ##, added by one of the methods above depending on the type of the line.

Then there is
https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L57
who adds a PHP_EOL in the beggining, in the end and after each message

[TomasVotruba]

What about inside https://github.com/Symplify/Symplify/blob/master/packages/ChangelogLinker/src/ChangelogDumper.php#L69

[orklah]

Thanks, that's the part I missed.

I didn't thought the content variable was reformatted after.

[orklah]

I'll close this issue. the specific cases are globally fixed in #1661. The discussion can be continued there.