Skip to content

Commit

Permalink
Change how command results are shown for long lists of paths
Browse files Browse the repository at this point in the history
Instead of printing all the paths, for any list of >3 paths we now print the
the number of paths looked at, the `include` globs for the command, and the
first 2 paths.
  • Loading branch information
autarch committed Dec 16, 2023
1 parent cd426f7 commit d53919c
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 11 deletions.
125 changes: 125 additions & 0 deletions precious-core/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub struct LintOrTidyCommand {
pub name: String,
typ: LintOrTidyCommandType,
includer: Matcher,
include: Vec<String>,
excluder: Matcher,
invoke: Invoke,
working_dir: WorkingDir,
Expand Down Expand Up @@ -240,6 +241,7 @@ impl LintOrTidyCommand {
name: params.name,
typ: params.typ,
includer: MatcherBuilder::new(&root).with(&params.include)?.build()?,
include: params.include,
excluder: MatcherBuilder::new(&root).with(&params.exclude)?.build()?,
invoke: params.invoke,
working_dir: params.working_dir,
Expand Down Expand Up @@ -665,6 +667,36 @@ impl LintOrTidyCommand {
Ok(cmd)
}

pub(crate) fn files_summary(&self, paths: &[&Path]) -> String {
let all = paths
.iter()
.sorted()
.map(|p| p.to_string_lossy().to_string())
.join(" ");
if paths.len() <= 3 {
return all;
}

match self.invoke {
Invoke::Once => {
let initial = paths
.iter()
.take(2)
.sorted()
.map(|p| p.to_string_lossy())
.join(" ");
format!(
"{} files matching {}, starting with {}",
paths.len(),
self.include.join(" "),
initial
)
}
Invoke::PerDir => format!("{} files in {}", paths.len(), all,),
Invoke::PerFile => format!("{} files: {}", paths.len(), all,),
}
}

fn paths_were_changed(&self, prev: PathMetadata) -> Result<bool> {
for (prev_file, prev_meta) in &prev.path_map {
debug!("Checking {} for changes", prev_file.display());
Expand Down Expand Up @@ -786,6 +818,7 @@ mod tests {
name: String::new(),
typ: LintOrTidyCommandType::Lint,
includer: matcher(&[])?,
include: vec![],
excluder: matcher(&[])?,
invoke: Invoke::PerFile,
working_dir: WorkingDir::Root,
Expand Down Expand Up @@ -1614,4 +1647,96 @@ mod tests {

Ok(())
}

#[test]
#[parallel]
fn files_summary() -> Result<()> {
struct TestCase {
invoke: Invoke,
include: Vec<String>,
paths: Vec<&'static Path>,
expect: &'static str,
}

let tests = vec![
TestCase {
invoke: Invoke::Once,
include: vec![String::from("**/*.go")],
paths: vec![Path::new("foo.go")],
expect: "foo.go",
},
TestCase {
invoke: Invoke::Once,
include: vec![String::from("**/*.go")],
paths: ["foo.go", "bar.go"]
.iter()
.map(Path::new)
.collect::<Vec<_>>(),
expect: "bar.go foo.go",
},
TestCase {
invoke: Invoke::Once,
include: vec![String::from("**/*.go")],
paths: ["foo.go", "bar.go", "baz.go"]
.iter()
.map(Path::new)
.collect::<Vec<_>>(),
expect: "bar.go baz.go foo.go",
},
TestCase {
invoke: Invoke::Once,
include: vec![String::from("**/*.go")],
paths: ["foo.go", "bar.go", "baz.go", "quux.go"]
.iter()
.map(Path::new)
.collect::<Vec<_>>(),
expect: "4 files matching **/*.go, starting with bar.go foo.go",
},
TestCase {
invoke: Invoke::Once,
include: ["**/*.go", "!food.go"]
.iter()
.map(|s| s.to_string())
.collect::<Vec<_>>(),
paths: ["foo.go", "bar.go", "baz.go", "quux.go"]
.iter()
.map(Path::new)
.collect::<Vec<_>>(),
expect: "4 files matching **/*.go !food.go, starting with bar.go foo.go",
},
TestCase {
invoke: Invoke::PerDir,
include: ["**/*.go", "!food.go"]
.iter()
.map(|s| s.to_string())
.collect::<Vec<_>>(),
paths: vec![Path::new("foo")],
expect: "foo",
},
TestCase {
invoke: Invoke::PerFile,
include: ["**/*.go", "!food.go"]
.iter()
.map(|s| s.to_string())
.collect::<Vec<_>>(),
paths: vec![Path::new("foo.go")],
expect: "foo.go",
},
];

for t in tests.into_iter() {
let command = LintOrTidyCommand {
name: String::from("Test"),
invoke: t.invoke,
include: t.include,
..default_command()?
};

{
assert_eq!(command.files_summary(&t.paths), t.expect);
}
}

Ok(())
}
}
22 changes: 11 additions & 11 deletions precious-core/src/precious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,43 +761,43 @@ impl LintOrTidyRunner {
Ok(Some(TidyOutcome::Changed)) => {
if !s.quiet {
println!(
"{} Tidied by {}: [{}]",
"{} Tidied by {}: {}",
s.chars.tidied,
t.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
t.files_summary(files),
);
}
Some(Ok(()))
}
Ok(Some(TidyOutcome::Unchanged)) => {
if !s.quiet {
println!(
"{} Unchanged by {}: [{}]",
"{} Unchanged by {}: {}",
s.chars.unchanged,
t.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
t.files_summary(files),
);
}
Some(Ok(()))
}
Ok(Some(TidyOutcome::Unknown)) => {
if !s.quiet {
println!(
"{} Maybe changed by {}: [{}]",
"{} Maybe changed by {}: {}",
s.chars.unknown,
t.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
t.files_summary(files),
);
}
Some(Ok(()))
}
Ok(None) => None,
Err(e) => {
println!(
"{} Error from {}: [{}]",
"{} Error from {}: {}",
s.chars.execution_error,
t.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
t.files_summary(files),
);
Some(Err(ActionFailure {
error: format!("{e:#}"),
Expand Down Expand Up @@ -825,7 +825,7 @@ impl LintOrTidyRunner {
"{} Passed {}: {}",
s.chars.lint_free,
l.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
l.files_summary(files),
);
}
Some(Ok(()))
Expand All @@ -834,7 +834,7 @@ impl LintOrTidyRunner {
"{} Failed {}: {}",
s.chars.lint_dirty,
l.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
l.files_summary(files),
);
if let Some(s) = lo.stdout {
println!("{s}");
Expand Down Expand Up @@ -869,7 +869,7 @@ impl LintOrTidyRunner {
"{} error {}: {}",
s.chars.execution_error,
l.name,
files.iter().map(|p| p.to_string_lossy()).join(" "),
l.files_summary(files),
);
Some(Err(ActionFailure {
error: format!("{e:#}"),
Expand Down

0 comments on commit d53919c

Please sign in to comment.