search

rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
5.95k stars 876 forks source link

Enhance diff_check with more unicode et al #5884

Open calebcartwright opened 1 year ago

calebcartwright commented 1 year ago

Follow up action item from a topic I raised in https://github.com/rust-lang/rustfmt/pull/5864#issuecomment-1676425492 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:

https://github.com/rust-lang/rustfmt/blob/b069aac44ddfdb70d55d9ae40695be44515e5bb0/ci/check_diff.sh#L170-L192

raymondyegon commented 1 year ago

@rustbot claim

ytmimi commented 1 year ago

@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.

https://github.com/rust-lang/rustfmt/blob/f89cd3c1f3feffaa960e13c7bec07a6163d18fd3/ci/check_diff.sh#L59-L62

calebcartwright commented 1 year ago

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 commented 1 year ago

@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.