Replace DOS line endings with LF#49
Conversation
DOS line endings are not currently stripped from the wp-config files, which results in extra lines being added to the modified file. Fixes wp-cli/config-command#160
|
@xknown This is not something that can be solved with a simple What the code does with your changes seems worse than before. Imagine the scenario where you have two regular blank lines in the standard notation for servers: The logic with this PR will first replace the middle part of that sequence, breaking the two individual blank lines. Then, as it completes, the final result will be this: So, for the more common configurations, it will end up turning two blank lines into four blank lines. To solve this properly, we would need to switch away from |
That's not really the case though, because the |
|
@xknown Indeed, you're right, my bad. I tested this in 3v4l.org and it seems to reliably work with all constellations. |
| "// this is a demo\r\n", | ||
| "define( 'DB_NAME', '' );\n", | ||
| "define( 'DB_HOST', '' );\r\n", | ||
| "define( 'DB_USER', '' );\n\r", | ||
| "define( 'DB_COLLATE', '');\n", |
There was a problem hiding this comment.
Can we add double blank lines in multiple constellations here as well just to fully demonstrate it works without splitting the combinations?
There was a problem hiding this comment.
Added them in dd6f0f9, however I think there's another bug that happens when the provided config file has a multiline string containing any of the replaced characters. In theory, we shouldn't be touching them. To fix it properly, we might require a proper PHP parser though, not sure if it already exists somewhere in wp-cli?
There was a problem hiding this comment.
No, we don't have a PHP Parser around, we tried to avoid adding dependencies to this to make sure it doesn't randomly break from conflicts.
There was a problem hiding this comment.
however I think there's another bug that happens when the provided config file has a multiline string containing any of the replaced characters.
Can you open a separate issue for this?
DOS line endings are not currently stripped from the wp-config files, which results in extra lines being added to the modified file.
Fixes wp-cli/config-command#160
Related: wp-cli/wp-cli#5859