Skip to content

Commit 3d5d6d2

Browse files
committed
Adjust x fmt printed output.
Currently, `x fmt` can print two lists of files. - The untracked files that are skipped. Always done if within a git repo. - The modified files that are formatted. But if you run with `--all` (or with `GITHUB_ACTIONS=true`) it doesn't print anything about which files are formatted. This commit increases consistency. - The formatted/checked files are now always printed. And it makes it clear why a file was formatted, e.g. with "modified". - It uses the same code for both untracked files and formatted/checked files. This means that now if there are a lot of untracked files just the number will be printed, which is like the old behaviour for modified files. Example output: ``` fmt: skipped 31 untracked files fmt: formatted modified file compiler/rustc_mir_transform/src/instsimplify.rs fmt: formatted modified file compiler/rustc_mir_transform/src/validate.rs fmt: formatted modified file library/core/src/ptr/metadata.rs fmt: formatted modified file src/bootstrap/src/core/build_steps/format.rs ``` or (with `--all`): ``` fmt: checked 3148 files ```
1 parent e98740a commit 3d5d6d2

File tree

1 file changed

+45
-29
lines changed

1 file changed

+45
-29
lines changed

src/bootstrap/src/core/build_steps/format.rs

+45-29
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::collections::VecDeque;
99
use std::path::{Path, PathBuf};
1010
use std::process::{Command, Stdio};
1111
use std::sync::mpsc::SyncSender;
12+
use std::sync::Mutex;
1213

1314
fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl FnMut(bool) -> bool {
1415
let mut cmd = Command::new(rustfmt);
@@ -97,6 +98,21 @@ struct RustfmtConfig {
9798
ignore: Vec<String>,
9899
}
99100

101+
// Prints output describing a collection of paths, with lines such as "formatted modified file
102+
// foo/bar/baz" or "skipped 20 untracked files".
103+
fn print_paths(verb: &str, adjective: Option<&str>, paths: &[String]) {
104+
let len = paths.len();
105+
let adjective =
106+
if let Some(adjective) = adjective { format!("{adjective} ") } else { String::new() };
107+
if len <= 10 {
108+
for path in paths {
109+
println!("fmt: {verb} {adjective}file {path}");
110+
}
111+
} else {
112+
println!("fmt: {verb} {len} {adjective}files");
113+
}
114+
}
115+
100116
pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
101117
if !paths.is_empty() {
102118
eprintln!("fmt error: path arguments are not accepted");
@@ -149,6 +165,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
149165
Err(_) => false,
150166
};
151167

168+
let mut adjective = None;
152169
if git_available {
153170
let in_working_tree = match build
154171
.config
@@ -172,45 +189,27 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
172189
.arg("-z")
173190
.arg("--untracked-files=normal"),
174191
);
175-
let untracked_paths = untracked_paths_output.split_terminator('\0').filter_map(
176-
|entry| entry.strip_prefix("?? "), // returns None if the prefix doesn't match
177-
);
178-
let mut untracked_count = 0;
192+
let untracked_paths: Vec<_> = untracked_paths_output
193+
.split_terminator('\0')
194+
.filter_map(
195+
|entry| entry.strip_prefix("?? "), // returns None if the prefix doesn't match
196+
)
197+
.map(|x| x.to_string())
198+
.collect();
199+
print_paths("skipped", Some("untracked"), &untracked_paths);
200+
179201
for untracked_path in untracked_paths {
180-
println!("fmt: skip untracked path {untracked_path} during rustfmt invocations");
181202
// The leading `/` makes it an exact match against the
182203
// repository root, rather than a glob. Without that, if you
183204
// have `foo.rs` in the repository root it will also match
184205
// against anything like `compiler/rustc_foo/src/foo.rs`,
185206
// preventing the latter from being formatted.
186-
untracked_count += 1;
187-
fmt_override.add(&format!("!/{untracked_path}")).expect(untracked_path);
207+
fmt_override.add(&format!("!/{untracked_path}")).expect(&untracked_path);
188208
}
189209
if !all {
210+
adjective = Some("modified");
190211
match get_modified_rs_files(build) {
191212
Ok(Some(files)) => {
192-
if files.len() <= 10 {
193-
for file in &files {
194-
println!("fmt: formatting modified file {file}");
195-
}
196-
} else {
197-
let pluralized = |count| if count > 1 { "files" } else { "file" };
198-
let untracked_msg = if untracked_count == 0 {
199-
"".to_string()
200-
} else {
201-
format!(
202-
", skipped {} untracked {}",
203-
untracked_count,
204-
pluralized(untracked_count),
205-
)
206-
};
207-
println!(
208-
"fmt: formatting {} modified {}{}",
209-
files.len(),
210-
pluralized(files.len()),
211-
untracked_msg
212-
);
213-
}
214213
for file in files {
215214
fmt_override.add(&format!("/{file}")).expect(&file);
216215
}
@@ -278,16 +277,33 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
278277
}
279278
});
280279

280+
let formatted_paths = Mutex::new(Vec::new());
281+
let formatted_paths_ref = &formatted_paths;
281282
walker.run(|| {
282283
let tx = tx.clone();
283284
Box::new(move |entry| {
285+
let cwd = std::env::current_dir();
284286
let entry = t!(entry);
285287
if entry.file_type().map_or(false, |t| t.is_file()) {
288+
formatted_paths_ref.lock().unwrap().push({
289+
// `into_path` produces an absolute path. Try to strip `cwd` to get a shorter
290+
// relative path.
291+
let mut path = entry.clone().into_path();
292+
if let Ok(cwd) = cwd {
293+
if let Ok(path2) = path.strip_prefix(cwd) {
294+
path = path2.to_path_buf();
295+
}
296+
}
297+
path.display().to_string()
298+
});
286299
t!(tx.send(entry.into_path()));
287300
}
288301
ignore::WalkState::Continue
289302
})
290303
});
304+
let mut paths = formatted_paths.into_inner().unwrap();
305+
paths.sort();
306+
print_paths(if check { "checked" } else { "formatted" }, adjective, &paths);
291307

292308
drop(tx);
293309

0 commit comments

Comments
 (0)