Skip to content

fix not to overwrite on other platforms #3971

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 20, 2019

Conversation

rchaser53
Copy link
Contributor

fix: #3956

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

} else {
let should_insert = !mods_outside_ast
.iter()
.any(|(outside_path, _, _)| outside_path == &path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: sorry for being picky, but I would rather write this using all instead of negation + any:

                    let should_insert = mods_outside_ast
                        .iter()
                        .all(|(outside_path, _, _)| outside_path != &path);

@topecongiro topecongiro merged commit 83f44e2 into rust-lang:master Dec 20, 2019
@rchaser53 rchaser53 deleted the issue-3956 branch December 20, 2019 02:19
@YruamaLairba
Copy link

Hi, does cargo-fmt 1.4.11 integrate this patch ? I encounter a bug very similar to #3956

@ytmimi
Copy link
Contributor

ytmimi commented Apr 1, 2022

#4100 implements a similar fix, and I can't reproduce the error on the latest master rustfmt 1.4.38-nightly (5ff7b632 2022-03-29)

@ytmimi
Copy link
Contributor

ytmimi commented Apr 1, 2022

@calebcartwright do you want me to backport the tests to prevent a regression?

@calebcartwright
Copy link
Member

@calebcartwright do you want me to backport the tests to prevent a regression?

Sure, that'd be great!

@ytmimi
Copy link
Contributor

ytmimi commented Apr 2, 2022

Will get on that soon!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contents of module with different name gets overwritten on other platform
6 participants