- Notifications
You must be signed in to change notification settings - Fork262
fix(commit_parser): strip DOS carriage-returns in commits#956
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(commit_parser): strip DOS carriage-returns in commits#956
Uh oh!
There was an error while loading.Please reload this page.
Conversation
bef4166 tob46a193CompareUh oh!
There was an error while loading.Please reload this page.
codejedi365 commentedJun 17, 2024
I know this seems a bit like a crazy idea but maybe we don't strip the Maybe this actually just bleeds into the code that handles the rendering rather than the content of the commit message. So it is likely your modification to remove them essentially is correct, then the during the writing of the file is when we apply the |
309ef0f to35fb0f2Comparewyardley commentedJun 17, 2024
I don’t think that’s necessary? The rest of the codebaseand most of the generated changelog is already using regular UNIX style CRs and it doesn’t sound like that’s caused any issues for people? |
wyardley commentedJun 17, 2024
Also, Ithink Python in normal cases will honor Either way, I think removing the mixed in ones should work for most. For sure IDEs and common text editors on Windows will render it fine, and I think other editors will as well, whereas with the btw, I don’t think these were introduced via commits on windows machines. I think maybe it’s how the strings are coming back from PyGithub, but could be wrong. |
codejedi365 commentedJun 17, 2024
I noticed that if someone uses the GitHub editor to make a commit that the commit message from the text area in the webUI will create commits with |
codejedi365 left a comment
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.
I like the second way as stated above. Thanks for the effort
wyardley commentedJun 17, 2024
Updated.
Nowthis makes a lot of sense, and that's probably the reason. |
35fb0f2 tofbbd066CompareThe default template can result in mixed (UNIX / DOS style) carriagereturns in the generated changelog. Use a string replace in the commitparser to strip the DOS CRs ("\r"). This is only needed in the case whenwe are _not_ byte decoding.Fixespython-semantic-release#955fbbd066 to75d5d30Compare
Uh oh!
There was an error while loading.Please reload this page.
The default template can result in mixed (UNIX / DOS style) carriage returns in the generated changelog. Use a string replace in the commit parser to strip the DOS CRs ("\r"). While it's possible to do the replacement in the template, IMO, this is the better / correct way to fix it.
I checked, and seems like maybe it's only needed when we'renot doing a byte decode (so the case where
mis a raw string)? I can add it back to both if you'd like, though.Fixes#955
Note: this may result in some diffs in generated changelogs.
See also#952
I'm happy to adjust existing tests in a way that would catch this and / or add a new test if someone can give me a general pointer on the best way / best place to do this. Also feel free to suggest a terser or more idiomatic way to do this.