Skip to content
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

fix(fmt): eof newline #2690

Merged
merged 1 commit into from Aug 10, 2022
Merged

fix(fmt): eof newline #2690

merged 1 commit into from Aug 10, 2022

Conversation

rkrasiuk
Copy link
Collaborator

address #2667

@rkrasiuk rkrasiuk added T-bug Type: bug Cmd-forge-fmt Command: forge fmt labels Aug 10, 2022
Copy link
Contributor

@jpopesculian jpopesculian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this regression was my bad! thanks for fixing it. im wondering though if we can just change this line https://github.com/foundry-rs/foundry/blob/master/fmt/src/formatter.rs#L3492 to actual compare the strings and use actual correct line endings in windows but that would be a slightly more intense story

@jpopesculian
Copy link
Contributor

jpopesculian commented Aug 10, 2022

this regression was my bad! thanks for fixing it. im wondering though if we can just change this line https://github.com/foundry-rs/foundry/blob/master/fmt/src/formatter.rs#L3492 to actual compare the strings and use actual correct line endings in windows but that would be a slightly more intense story

i thought about it again and it seems even the standard library doesn't care about inserting carriage returns, so i think this is fine the way that it is! https://doc.rust-lang.org/std/macro.writeln.html Not to mention having platform differences may cause a version control horror...

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@mattsse mattsse merged commit 6c96bb8 into master Aug 10, 2022
@mattsse mattsse deleted the rkrasiuk/fmt-eof-newline branch August 10, 2022 13:00
iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-fmt Command: forge fmt T-bug Type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants