Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Conversation

@wyardley
Copy link
Contributor

@wyardleywyardley commentedJun 15, 2024
edited
Loading

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 wherem is 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.

@wyardleywyardleyforce-pushed thewyardley/issues/955/fix branch 3 times, most recently frombef4166 tob46a193CompareJune 15, 2024 15:03
@codejedi365codejedi365 self-requested a reviewJune 15, 2024 16:40
@codejedi365
Copy link
Contributor

I know this seems a bit like a crazy idea but maybe we don't strip the\r out but instead format the changelog with\r\n or windows style, because then it is always readable cross-platform. If we remove the\r then we restrict it only to Linux machines for reviewing the changelog and it does not work inheritly on windows. However the reverse will work on both.

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\r\n's.

@codejedi365codejedi365force-pushed thewyardley/issues/955/fix branch from309ef0f to35fb0f2CompareJune 17, 2024 06:12
@wyardley
Copy link
ContributorAuthor

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
Copy link
ContributorAuthor

Also, Ithink Python in normal cases will honoros.linesep when writing files, though in this case since we’re using Jinja, the behavior could be slightly different.

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\r\n you’ll see^Ms in the file (as you do now insome lines on this project’s changelog).

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
Copy link
Contributor

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/r/n. That is more likely where those odd ones are coming from in our changelog.

Copy link
Contributor

@codejedi365codejedi365 left a 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 reacted with thumbs up emoji
@wyardley
Copy link
ContributorAuthor

I like the second way as stated above. Thanks for the effort

Updated.

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 /r/n.

Nowthis makes a lot of sense, and that's probably the reason.

@wyardleywyardleyforce-pushed thewyardley/issues/955/fix branch from35fb0f2 tofbbd066CompareJune 17, 2024 19:00
The 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#955
@codejedi365codejedi365force-pushed thewyardley/issues/955/fix branch fromfbbd066 to75d5d30CompareJune 17, 2024 19:03
@codejedi365codejedi365 merged commit0b005df intopython-semantic-release:masterJun 18, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@codejedi365codejedi365codejedi365 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Inconsistent carriage returns in generated changelogs

2 participants

@wyardley@codejedi365

[8]ページ先頭

©2009-2025 Movatter.jp