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

Streamline x fmt and improve its output #125699

Merged
merged 5 commits into from
May 30, 2024
Merged

Streamline x fmt and improve its output #125699

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5cf198d
Remove path choice from `x fmt` and add `--all` option.
nnethercote May 28, 2024
e98740a
Clarify `x fmt` messages.
nnethercote May 29, 2024
3d5d6d2
Adjust `x fmt` printed output.
nnethercote May 29, 2024
a22cfcc
Rename `fmt_override`.
nnethercote May 29, 2024
4dec0a0
Clarify the closure in `rustfmt`.
nnethercote May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Clarify x fmt messages. 
- Precede them all with `fmt` so it's clear where they are coming from.
- Use `error:` and `warning:` when appropriate.
- Print warnings to stderr instead of stdout
  • Loading branch information
@nnethercote
nnethercote committed May 29, 2024
commit e98740aa0daa288b44925864a487fab7d4c536d5
33 changes: 16 additions & 17 deletions src/bootstrap/src/core/build_steps/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F
let status = cmd.wait().unwrap();
if !status.success() {
eprintln!(
"Running `{}` failed.\nIf you're running `tidy`, \
try again with `--bless`. Or, if you just want to format \
code, run `./x.py fmt` instead.",
"fmt error: Running `{}` failed.\nIf you're running `tidy`, \
try again with `--bless`. Or, if you just want to format \
code, run `./x.py fmt` instead.",
cmd_debug,
);
crate::exit!(1);
Expand Down Expand Up @@ -99,7 +99,7 @@ struct RustfmtConfig {

pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
if !paths.is_empty() {
eprintln!("path arguments are not accepted");
eprintln!("fmt error: path arguments are not accepted");
crate::exit!(1);
};
if build.config.dry_run() {
Expand All @@ -118,8 +118,8 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
let matcher = builder.build().unwrap();
let rustfmt_config = build.src.join("rustfmt.toml");
if !rustfmt_config.exists() {
eprintln!("Not running formatting checks; rustfmt.toml does not exist.");
eprintln!("This may happen in distributed tarballs.");
eprintln!("fmt error: Not running formatting checks; rustfmt.toml does not exist.");
eprintln!("fmt error: This may happen in distributed tarballs.");
return;
}
let rustfmt_config = t!(std::fs::read_to_string(&rustfmt_config));
Expand All @@ -133,7 +133,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
// any files that aren't explicitly mentioned. No bueno! Maybe there's a way to combine
// explicit whitelisted entries and traversal of unmentioned files, but for now just
// forbid such entries.
eprintln!("`!`-prefixed entries are not supported in rustfmt.toml, sorry");
eprintln!("fmt error: `!`-prefixed entries are not supported in rustfmt.toml, sorry");
crate::exit!(1);
} else {
fmt_override.add(&format!("!{ignore}")).expect(&ignore);
Expand Down Expand Up @@ -177,7 +177,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
);
let mut untracked_count = 0;
for untracked_path in untracked_paths {
println!("skip untracked path {untracked_path} during rustfmt invocations");
println!("fmt: skip untracked path {untracked_path} during rustfmt invocations");
// The leading `/` makes it an exact match against the
// repository root, rather than a glob. Without that, if you
// have `foo.rs` in the repository root it will also match
Expand All @@ -191,7 +191,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
Ok(Some(files)) => {
if files.len() <= 10 {
for file in &files {
println!("formatting modified file {file}");
println!("fmt: formatting modified file {file}");
}
} else {
let pluralized = |count| if count > 1 { "files" } else { "file" };
Expand All @@ -205,7 +205,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
)
};
println!(
"formatting {} modified {}{}",
"fmt: formatting {} modified {}{}",
files.len(),
pluralized(files.len()),
untracked_msg
Expand All @@ -217,24 +217,23 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
}
Ok(None) => {}
Err(err) => {
println!(
"WARN: Something went wrong when running git commands:\n{err}\n\
Falling back to formatting all files."
);
eprintln!("fmt warning: Something went wrong running git commands:");
eprintln!("fmt warning: {err}");
eprintln!("fmt warning: Falling back to formatting all files.");
}
}
}
} else {
println!("Not in git tree. Skipping git-aware format checks");
eprintln!("fmt: warning: Not in git tree. Skipping git-aware format checks");
}
} else {
println!("Could not find usable git. Skipping git-aware format checks");
eprintln!("fmt: warning: Could not find usable git. Skipping git-aware format checks");
}

let fmt_override = fmt_override.build().unwrap();

let rustfmt_path = build.initial_rustfmt().unwrap_or_else(|| {
eprintln!("./x.py fmt is not supported on this channel");
eprintln!("fmt error: `x fmt` is not supported on this channel");
crate::exit!(1);
});
assert!(rustfmt_path.exists(), "{}", rustfmt_path.display());
Expand Down