-
-
Notifications
You must be signed in to change notification settings - Fork 313
Handle CRLF line breaks in the patch parser #91
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
Conversation
This fixes an issue where file creation wasn't occurring on Windows due to `startPath !== "/dev/null"` because `startPath` had a trailing `\r`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking, I could've written a more focused test that does
expect(parsePatch(crlfLineBreaks)[0].type).toBe("file creation")
Let me know if you'd prefer that instead of a snapshot test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there's any implications from using .trim() as opposed to trimming from the end e.g.
const trimEnd = str => str.replace(/\s+$/, '')
This looks great, thanks! Sorry it took me a while to get around to it.
published in patch-package@6.0.0-11 Thanks again! 🎉
No problem, thank you for reviewing and releasing! 🙂
Uh oh!
There was an error while loading. Please reload this page.
I noticed that if a patch file was checked out on Windows and it contained CRLF linebreaks, then
patch-packagewould fail on file creation because this condition would be false due tostartPathcontaining a trailing\r(carriage return) character: https://github.com/ds300/patch-package/blob/54a168b/src/patch/parse.ts#L297After some investigation, it turned out that this was happening because https://github.com/ds300/patch-package/blob/54a168b/src/patch/parse.ts#L387 was splitting patch file contents on
\nand so every line contained a trailing\r(削除) Interestingly, I couldn't getThanks to CI, I've now managed to ensure all tests pass.yarn test --runInBandto pass locally even on a fresh checkout (without my changes) 🙁 (削除ここまで)