Conversation
|
I've updated directory separator to be normalized so it can works on windows. |
|
Ready to merge 👍 the pending CI needs to be replaced with new defined action. |
|
@ruudk Ready 👍 |
|
@ruudk do you think it is mergeable now? I only update the test part with no behaviour changed, just so it can run unit test on windows local dev. Thank you. |
| yield [ | ||
| "↳ <href=phpstorm://open?file=/www/project/src/Core/Admin/Controller/Dashboard/User/AddUserController.php&line=20>src/Core/Admin/.../User/AddUserController.php:20</>\n", | ||
| self::isWindows() | ||
| ? "↳ <href=phpstorm://open?file=/www/project/src/Core/Admin/Controller/Dashboard/User/AddUserController.php&line=20>src/Core/Admin/Controller/Dashboard/User/AddUserController.php:20</>\n" |
There was a problem hiding this comment.
On Windows, these paths don't make any sense. They should be using backslashes.
There was a problem hiding this comment.
This on IDE link works even on Windows, the data provider test may need change to use relative path that doesn't use root drive path as directory separator use / on relative path
There was a problem hiding this comment.
But when the formatter presents errors, on windows, you would like to see windows paths, right?
Why would you want to see linux paths there?
There was a problem hiding this comment.
The link should test show relative path, which formatter doesn't use DIRECTORY_SEPARATOR, it uses /.
The different is on windows, /.../ converted to parent directory, as this ternary use, I will check if it is possible to use relative path instead of include root drive
There was a problem hiding this comment.
@ruudk I've updated the test to show example of DIRECTORY_SEPARATOR based on operating system 👍
should be ok now 👍
|
Thanks, could you rebase this with main? Thanks, |
|
@ruudk it already synced with latest main branch, the pending CI is because it add new Windows CI, and that's need to be enabled. |
2 participants
This PR add windows on CI so error on windows can be detected.