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.

Sign up for GitHub

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

Jump to bottom

Enhance diff_check with more unicode et al #5884

Open
calebcartwright opened this issue Aug 13, 2023 · 4 comments
Open

Enhance diff_check with more unicode et al #5884

calebcartwright opened this issue Aug 13, 2023 · 4 comments
Assignees
@raymondyegon
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted

Comments

@calebcartwright
Copy link
Member

calebcartwright commented Aug 13, 2023 •
edited by rustbot
Loading

Follow up action item from a topic I raised in #5864 (comment) relative to adding some additional test repos to improve our coverage of string formatting with various characters and encodings.

One suggested repo to include is https://github.com/unicode-org/icu4x as well as some from https://github.com/unicode-rs

I opted to simply mention icu4x as part of this instead of directly adding it because I noticed that repo is using cargo make and I didn't have bandwidth to determine if that'd cause any issues/require any additional changes to our script.

New repos can be wired up by adding a call site with respective args here:

rustfmt/ci/check_diff.sh

Lines 170 to 192 in b069aac

check_repo "https://github.com/rust-lang/rust.git" rust-lang-rust
check_repo "https://github.com/rust-lang/cargo.git" cargo
check_repo "https://github.com/rust-lang/miri.git" miri
check_repo "https://github.com/rust-lang/rust-analyzer.git" rust-analyzer
check_repo "https://github.com/bitflags/bitflags.git" bitflags
check_repo "https://github.com/rust-lang/log.git" log
check_repo "https://github.com/rust-lang/mdBook.git" mdBook
check_repo "https://github.com/rust-lang/packed_simd.git" packed_simd
check_repo "https://github.com/rust-lang/rust-semverver.git" check_repo
check_repo "https://github.com/Stebalien/tempfile.git" tempfile
check_repo "https://github.com/rust-lang/futures-rs.git" futures-rs
check_repo "https://github.com/dtolnay/anyhow.git" anyhow
check_repo "https://github.com/dtolnay/thiserror.git" thiserror
check_repo "https://github.com/dtolnay/syn.git" syn
check_repo "https://github.com/serde-rs/serde.git" serde
check_repo "https://github.com/rust-lang/rustlings.git" rustlings
check_repo "https://github.com/rust-lang/rustup.git" rustup
check_repo "https://github.com/SergioBenitez/Rocket.git" Rocket
check_repo "https://github.com/rustls/rustls.git" rustls
check_repo "https://github.com/rust-lang/rust-bindgen.git" rust-bindgen
check_repo "https://github.com/hyperium/hyper.git" hyper
check_repo "https://github.com/actix/actix.git" actix
check_repo "https://github.com/denoland/deno.git" denoland_deno

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted labels Aug 13, 2023
@raymondyegon
Copy link

@rustbot claim

@ytmimi
Copy link
Contributor

ytmimi commented Aug 30, 2023

@calebcartwright I don't think the use of cargo make on any project we test would be an issue. The diff check job just runs rustfmt againsg all .rs files it finds in the repo.

rustfmt/ci/check_diff.sh

Lines 59 to 62 in f89cd3c

for i in `find . | grep "\.rs$"`
do
$1 --unstable-features --skip-children --check --color=always $config $i >> $2 2>/dev/null
done

@calebcartwright
Copy link
Member Author

Forgot we were running rustfmt directly. The only potential downside to this would be our ability to pull in any target repositories that needed 2018 edition & didn't have edition defined in a rustfmt.toml file.

Orthogonal to this particular issue, but at some point would like to think/talk through whether that has any implications once style_edition enters the ecosystem

@ytmimi
Copy link
Contributor

ytmimi commented Sep 11, 2023 •
edited
Loading

@calebcartwright good point. I guess if we landed support for cargo fmt to be passed individual files then that would mostly solve the edition issue. I'm nearly 100% sure style_edition is going to be implemented in such a way that we'll derive it from the --edition if you don't explicitly configure style_edition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@raymondyegon raymondyegon

Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@calebcartwright @ytmimi @raymondyegon