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

rustfmt.toml fixes #125429

Closed
wants to merge 2 commits into from
Closed

Conversation

nnethercote
Copy link
Contributor

Two small fixes for rustfmt.toml.

r? @GuillaumeGomez

I tried some `rustfmt.toml` changes and these files were changed under
`tests/run-make`:

    tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs
    tests/run-make/compiler-builtins/rmake.rs
    tests/run-make/cross-lang-lto-riscv-abi/rmake.rs
    tests/run-make/issue-26006/in/libc/lib.rs
    tests/run-make/issue-26006/in/time/lib.rs
    tests/run-make/print-cfg/rmake.rs
    tests/run-make/print-native-static-libs/rmake.rs
    tests/run-make/print-to-output/rmake.rs
    tests/run-make/rustdoc-scrape-examples-invalid-expr/src/lib.rs
    tests/run-make/rustdoc-scrape-examples-macros/src/lib.rs
    tests/run-make/rustdoc-scrape-examples-test/examples/ex.rs
    tests/run-make/thumb-none-qemu/example/src/main.rs
    tests/run-make/wasm-exceptions-nostd/src/arena_alloc.rs
    tests/run-make/wasm-exceptions-nostd/src/lib.rs
    tests/run-make/wasm-exceptions-nostd/src/panicking.rs
    tests/run-make/x86_64-fortanix-unknown-sgx-lvi/enclave/build.rs

Only the `rmake.rs` files should have been changed.

This commit adjust the rules to initially exclude all files under
`tests/run-make/`, not just `tests/run-make/*/*.rs`. Because there are
some `.rs` files at deeper depths. After that, the changes I get are to
only the desired files:

    tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs
    tests/run-make/compiler-builtins/rmake.rs
    tests/run-make/cross-lang-lto-riscv-abi/rmake.rs
    tests/run-make/print-cfg/rmake.rs
    tests/run-make/print-native-static-libs/rmake.rs
    tests/run-make/print-to-output/rmake.rs
This comment -- "by default we ignore everything in the repository" --
was added in rust-lang#65939 when rustfmt was first being introduced for this
repository and (briefly) every directory was ignored. Since then lots of
directories have opted in to formatting, so it is no longer true.
@rustbot rustbot added A-meta Area: Issues about the rust-lang/rust repository. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2024
@nnethercote
Copy link
Contributor Author

cc @Urgau @onur-ozkan

Comment on lines -21 to +20
"/tests/run-make/*/*.rs",
"/tests/run-make/*",
Copy link
Member

@Urgau Urgau May 23, 2024

Choose a reason for hiding this comment

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

This change doesn't work for me locally.

I changed my rustfmt.toml to yours, than changed the formatting inside a rmake.rs and than did ./x.py fmt and while the "formatting modified file ..." message was printed, the file wasn't formatted, without this change it is correctly formatted.

Note that we do some weird handling of rustfmt ignore list in bootstrap, it may be related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the behaviour of these rmake.rs rules is really weird.

  • I was seeing the original problem when I was explicitly listing files to format.
  • If you change the style in rustfmt.toml majorly (e.g. add imports_granularity = "Item") and then run x fmt, it should do nothing, because no files have changed. But it formats all the rmake.rs files.

Copy link
Member

Choose a reason for hiding this comment

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

In this case, it might be simpler to update how the rule is implemented in bootstrap to make it more straightforward I think.

@nnethercote
Copy link
Contributor Author

This PR is superseded by #125637.

@nnethercote nnethercote deleted the rustfmt-toml-fixes branch May 28, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues about the rust-lang/rust repository. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants