fix(commit_parser): strip DOS carriage-returns in commits #956
Conversation
bef4166 to
b46a193
Compare
|
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 to
35fb0f2
Compare
|
I don’t think that’s necessary? The rest of the codebase and 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? |
|
Also, I think 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. |
|
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 |
Updated.
Now this makes a lot of sense, and that's probably the reason. |
35fb0f2 to
fbbd066
Compare
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"). This is only needed in the case when
we are _not_ byte decoding.
Fixes python-semantic-release#955
fbbd066 to
75d5d30
Compare
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're not 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.