Detect line breaks in different OS #1894
Conversation
luissmg
changed the title
|
Hi. Adding the tests right now. Do you want me to add tests for every case that exists right now but just change the line break? |
|
@luissmg we definitely don't need that many tests :-) one or two should probably suffice. |
|
I thought just the same but wanted to confirm first :) |
|
@ljharb Sorry for taking so long on this. Added the tests! |
|
@luissmg altho it seems like the tests are failing |
|
@ljharb I was checking and this test that I added is exactly the same as the one above that is passing, excepting for the type of line break. Shouldn't it pass too or am I missing something? Edit: the other test with |
|
I suspect the failing test ends up with a mix of |
|
Hi. Would love to see this get merged 😄 Currently making me sad about my Windows machine! |
|
It needs a rebase and fixed tests. |
|
@ljharb @grahammcculloch Doing that this weekend! |
|
@luissmg any news? :) |
| countNewLinesBeforeContent = (child.raw.match(/^ *\n/g) || []).length; | ||
| countNewLinesAfterContent = (child.raw.match(/\n *$/g) || []).length; | ||
| countNewLinesBeforeContent = (child.raw.match(/^ *[\r\n|\r|\n]/g) || []).length; | ||
| countNewLinesAfterContent = (child.raw.match(/[\r\n|\r|\n] *$/g) || []).length; |
There was a problem hiding this comment.
I think you meant (\r\n|\r|\n) or [\r\n] since brackets will match anything inside, even pipes. The best regex for line endings is simply \r?\n. It will match both CRLF and LF. Matching only CR is not needed as it's not used anymore.
There was a problem hiding this comment.
Thank you so much for the tip! Will try to fix it this weekend
|
Hi guys. I will try to fix this in this weekend. Sorry for not replying any sooner. |
|
@luissmg are you still interested in completing this PR? |
|
Hi @ljharb! Sorry for letting this here in the air. I will try to push something this week. Will apply the suggestion from @sindresorhus and check if the tests are ok |
|
Hey @ljharb, how are you? I took a look into this today. can you check if it is okay? |
|
@luissmg unfortunately the tests don't fail, when your fix is removed. |
|
@ljharb Hmmmm you are right... Could be that this problem is not a problem anymore? |
|
It's entirely possible! |
ljharb
force-pushed
the
master
branch
4 times, most recently
from
deb3090 to
a32f61e
Compare
ljharb
force-pushed
the
master
branch
2 times, most recently
from
59af733 to
865ed16
Compare
ljharb
force-pushed
the
master
branch
4 times, most recently
from
069314a to
181c68f
Compare
ljharb
force-pushed
the
master
branch
2 times, most recently
from
380e32c to
51d342b
Compare
Check the following issue for reference.
#1893