Implement -q/--brief option (fixes #19) (#20)

* Implement -q/--brief option

* Optimization: stop analyzing the files as soon as there are any differences

* Unit tests for the stop_early parameter

* Simplify checks
This commit is contained in:
Olivier Tilloy
2024-03-19 11:45:06 +01:00
committed by GitHub
parent 62e10c6d6c
commit 4ed7ea1553
7 changed files with 394 additions and 40 deletions
+113 -11
View File
@@ -41,7 +41,12 @@ impl Mismatch {
}
// Produces a diff between the expected output and actual output.
fn make_diff(expected: &[u8], actual: &[u8], context_size: usize) -> Vec<Mismatch> {
fn make_diff(
expected: &[u8],
actual: &[u8],
context_size: usize,
stop_early: bool,
) -> Vec<Mismatch> {
let mut line_number_expected = 1;
let mut line_number_actual = 1;
let mut context_queue: VecDeque<&[u8]> = VecDeque::with_capacity(context_size);
@@ -191,6 +196,10 @@ fn make_diff(expected: &[u8], actual: &[u8], context_size: usize) -> Vec<Mismatc
line_number_actual += 1;
}
}
if stop_early && !results.is_empty() {
// Optimization: stop analyzing the files as soon as there are any differences
return results;
}
}
results.push(mismatch);
@@ -260,12 +269,16 @@ pub fn diff(
actual: &[u8],
actual_filename: &str,
context_size: usize,
stop_early: bool,
) -> Vec<u8> {
let mut output = format!("*** {expected_filename}\t\n--- {actual_filename}\t\n").into_bytes();
let diff_results = make_diff(expected, actual, context_size);
let diff_results = make_diff(expected, actual, context_size, stop_early);
if diff_results.is_empty() {
return Vec::new();
};
}
if stop_early {
return output;
}
for result in diff_results {
let mut line_number_expected = result.line_number_expected;
let mut line_number_actual = result.line_number_actual;
@@ -404,8 +417,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alef", &bet, &format!("{target}/alef"), 2);
let diff = diff(
&alef,
"a/alef",
&bet,
&format!("{target}/alef"),
2,
false,
);
File::create(&format!("{target}/ab.diff"))
.unwrap()
.write_all(&diff)
@@ -477,8 +496,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alef_", &bet, &format!("{target}/alef_"), 2);
let diff = diff(
&alef,
"a/alef_",
&bet,
&format!("{target}/alef_"),
2,
false,
);
File::create(&format!("{target}/ab_.diff"))
.unwrap()
.write_all(&diff)
@@ -553,8 +578,14 @@ mod tests {
};
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alefx", &bet, &format!("{target}/alefx"), 2);
let diff = diff(
&alef,
"a/alefx",
&bet,
&format!("{target}/alefx"),
2,
false,
);
File::create(&format!("{target}/abx.diff"))
.unwrap()
.write_all(&diff)
@@ -632,8 +663,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alefr", &bet, &format!("{target}/alefr"), 2);
let diff = diff(
&alef,
"a/alefr",
&bet,
&format!("{target}/alefr"),
2,
false,
);
File::create(&format!("{target}/abr.diff"))
.unwrap()
.write_all(&diff)
@@ -662,4 +699,69 @@ mod tests {
}
}
}
#[test]
fn test_stop_early() {
let from_filename = "foo";
let from = vec!["a", "b", "c", ""].join("\n");
let to_filename = "bar";
let to = vec!["a", "d", "c", ""].join("\n");
let context_size: usize = 3;
let diff_full = diff(
from.as_bytes(),
from_filename,
to.as_bytes(),
to_filename,
context_size,
false,
);
let expected_full = vec![
"*** foo\t",
"--- bar\t",
"***************",
"*** 1,3 ****",
" a",
"! b",
" c",
"--- 1,3 ----",
" a",
"! d",
" c",
"",
]
.join("\n");
assert_eq!(diff_full, expected_full.as_bytes());
let diff_brief = diff(
from.as_bytes(),
from_filename,
to.as_bytes(),
to_filename,
context_size,
true,
);
let expected_brief = vec!["*** foo\t", "--- bar\t", ""].join("\n");
assert_eq!(diff_brief, expected_brief.as_bytes());
let nodiff_full = diff(
from.as_bytes(),
from_filename,
from.as_bytes(),
to_filename,
context_size,
false,
);
assert!(nodiff_full.is_empty());
let nodiff_brief = diff(
from.as_bytes(),
from_filename,
from.as_bytes(),
to_filename,
context_size,
true,
);
assert!(nodiff_brief.is_empty());
}
}
+33 -5
View File
@@ -42,7 +42,7 @@ impl Mismatch {
}
// Produces a diff between the expected output and actual output.
fn make_diff(expected: &[u8], actual: &[u8]) -> Result<Vec<Mismatch>, DiffError> {
fn make_diff(expected: &[u8], actual: &[u8], stop_early: bool) -> Result<Vec<Mismatch>, DiffError> {
let mut line_number_expected = 1;
let mut line_number_actual = 1;
let mut results = Vec::new();
@@ -94,6 +94,10 @@ fn make_diff(expected: &[u8], actual: &[u8]) -> Result<Vec<Mismatch>, DiffError>
}
}
}
if stop_early && !results.is_empty() {
// Optimization: stop analyzing the files as soon as there are any differences
return Ok(results);
}
}
if !mismatch.actual.is_empty() || !mismatch.expected.is_empty() {
@@ -103,9 +107,13 @@ fn make_diff(expected: &[u8], actual: &[u8]) -> Result<Vec<Mismatch>, DiffError>
Ok(results)
}
pub fn diff(expected: &[u8], actual: &[u8]) -> Result<Vec<u8>, DiffError> {
pub fn diff(expected: &[u8], actual: &[u8], stop_early: bool) -> Result<Vec<u8>, DiffError> {
let mut output = Vec::new();
let diff_results = make_diff(expected, actual)?;
let diff_results = make_diff(expected, actual, stop_early)?;
if stop_early && !diff_results.is_empty() {
write!(&mut output, "\0").unwrap();
return Ok(output);
}
let mut lines_offset = 0;
for result in diff_results {
let line_number_expected: isize = result.line_number_expected as isize + lines_offset;
@@ -152,7 +160,7 @@ mod tests {
use super::*;
use pretty_assertions::assert_eq;
pub fn diff_w(expected: &[u8], actual: &[u8], filename: &str) -> Result<Vec<u8>, DiffError> {
let mut output = diff(expected, actual)?;
let mut output = diff(expected, actual, false)?;
writeln!(&mut output, "w {filename}").unwrap();
Ok(output)
}
@@ -161,7 +169,7 @@ mod tests {
fn test_basic() {
let from = b"a\n";
let to = b"b\n";
let diff = diff(from, to).unwrap();
let diff = diff(from, to, false).unwrap();
let expected = vec!["1c", "b", ".", ""].join("\n");
assert_eq!(diff, expected.as_bytes());
}
@@ -390,4 +398,24 @@ mod tests {
}
}
}
#[test]
fn test_stop_early() {
let from = vec!["a", "b", "c", ""].join("\n");
let to = vec!["a", "d", "c", ""].join("\n");
let diff_full = diff(from.as_bytes(), to.as_bytes(), false).unwrap();
let expected_full = vec!["2c", "d", ".", ""].join("\n");
assert_eq!(diff_full, expected_full.as_bytes());
let diff_brief = diff(from.as_bytes(), to.as_bytes(), true).unwrap();
let expected_brief = "\0".as_bytes();
assert_eq!(diff_brief, expected_brief);
let nodiff_full = diff(from.as_bytes(), from.as_bytes(), false).unwrap();
assert!(nodiff_full.is_empty());
let nodiff_brief = diff(from.as_bytes(), from.as_bytes(), true).unwrap();
assert!(nodiff_brief.is_empty());
}
}
+14 -3
View File
@@ -29,6 +29,7 @@ fn main() -> ExitCode {
context_count,
format,
report_identical_files,
brief,
} = parse_params(opts).unwrap_or_else(|error| {
eprintln!("{error}");
exit(2);
@@ -64,13 +65,14 @@ fn main() -> ExitCode {
};
// run diff
let result: Vec<u8> = match format {
Format::Normal => normal_diff::diff(&from_content, &to_content),
Format::Normal => normal_diff::diff(&from_content, &to_content, brief),
Format::Unified => unified_diff::diff(
&from_content,
&from.to_string_lossy(),
&to_content,
&to.to_string_lossy(),
context_count,
brief,
),
Format::Context => context_diff::diff(
&from_content,
@@ -78,13 +80,22 @@ fn main() -> ExitCode {
&to_content,
&to.to_string_lossy(),
context_count,
brief,
),
Format::Ed => ed_diff::diff(&from_content, &to_content).unwrap_or_else(|error| {
Format::Ed => ed_diff::diff(&from_content, &to_content, brief).unwrap_or_else(|error| {
eprintln!("{error}");
exit(2);
}),
};
io::stdout().write_all(&result).unwrap();
if brief && !result.is_empty() {
println!(
"Files {} and {} differ",
from.to_string_lossy(),
to.to_string_lossy()
);
} else {
io::stdout().write_all(&result).unwrap();
}
if result.is_empty() {
maybe_report_identical_files();
ExitCode::SUCCESS
+36 -8
View File
@@ -29,7 +29,7 @@ impl Mismatch {
}
// Produces a diff between the expected output and actual output.
fn make_diff(expected: &[u8], actual: &[u8]) -> Vec<Mismatch> {
fn make_diff(expected: &[u8], actual: &[u8], stop_early: bool) -> Vec<Mismatch> {
let mut line_number_expected = 1;
let mut line_number_actual = 1;
let mut results = Vec::new();
@@ -100,6 +100,10 @@ fn make_diff(expected: &[u8], actual: &[u8]) -> Vec<Mismatch> {
}
}
}
if stop_early && !results.is_empty() {
// Optimization: stop analyzing the files as soon as there are any differences
return results;
}
}
if !mismatch.actual.is_empty() || !mismatch.expected.is_empty() {
@@ -110,11 +114,15 @@ fn make_diff(expected: &[u8], actual: &[u8]) -> Vec<Mismatch> {
}
#[must_use]
pub fn diff(expected: &[u8], actual: &[u8]) -> Vec<u8> {
pub fn diff(expected: &[u8], actual: &[u8], stop_early: bool) -> Vec<u8> {
// See https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Normal.html
// for details on the syntax of the normal format.
let mut output = Vec::new();
let diff_results = make_diff(expected, actual);
let diff_results = make_diff(expected, actual, stop_early);
if stop_early && !diff_results.is_empty() {
write!(&mut output, "\0").unwrap();
return output;
}
for result in diff_results {
let line_number_expected = result.line_number_expected;
let line_number_actual = result.line_number_actual;
@@ -212,7 +220,7 @@ mod tests {
a.write_all(b"a\n").unwrap();
let mut b = Vec::new();
b.write_all(b"b\n").unwrap();
let diff = diff(&a, &b);
let diff = diff(&a, &b, false);
let expected = b"1c1\n< a\n---\n> b\n".to_vec();
assert_eq!(diff, expected);
}
@@ -265,7 +273,7 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff = diff(&alef, &bet);
let diff = diff(&alef, &bet, false);
File::create(&format!("{target}/ab.diff"))
.unwrap()
.write_all(&diff)
@@ -357,7 +365,7 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff = diff(&alef, &bet);
let diff = diff(&alef, &bet, false);
File::create(&format!("{target}/abn.diff"))
.unwrap()
.write_all(&diff)
@@ -431,7 +439,7 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff = diff(&alef, &bet);
let diff = diff(&alef, &bet, false);
File::create(&format!("{target}/ab_.diff"))
.unwrap()
.write_all(&diff)
@@ -509,7 +517,7 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff = diff(&alef, &bet);
let diff = diff(&alef, &bet, false);
File::create(&format!("{target}/abr.diff"))
.unwrap()
.write_all(&diff)
@@ -538,4 +546,24 @@ mod tests {
}
}
}
#[test]
fn test_stop_early() {
let from = vec!["a", "b", "c"].join("\n");
let to = vec!["a", "d", "c"].join("\n");
let diff_full = diff(from.as_bytes(), to.as_bytes(), false);
let expected_full = vec!["2c2", "< b", "---", "> d", ""].join("\n");
assert_eq!(diff_full, expected_full.as_bytes());
let diff_brief = diff(from.as_bytes(), to.as_bytes(), true);
let expected_brief = "\0".as_bytes();
assert_eq!(diff_brief, expected_brief);
let nodiff_full = diff(from.as_bytes(), from.as_bytes(), false);
assert!(nodiff_full.is_empty());
let nodiff_brief = diff(from.as_bytes(), from.as_bytes(), true);
assert!(nodiff_brief.is_empty());
}
}
+57
View File
@@ -26,6 +26,7 @@ pub struct Params {
pub format: Format,
pub context_count: usize,
pub report_identical_files: bool,
pub brief: bool,
}
pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params, String> {
@@ -40,6 +41,7 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
let mut format = None;
let mut context_count = 3;
let mut report_identical_files = false;
let mut brief = false;
while let Some(param) = opts.next() {
if param == "--" {
break;
@@ -58,6 +60,10 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
report_identical_files = true;
continue;
}
if param == "-q" || param == "--brief" {
brief = true;
continue;
}
let p = osstr_bytes(&param);
if p.first() == Some(&b'-') && p.get(1) != Some(&b'-') {
let mut bit = p[1..].iter().copied().peekable();
@@ -140,6 +146,7 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
format,
context_count,
report_identical_files,
brief,
})
}
@@ -158,6 +165,7 @@ mod tests {
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: false,
}),
parse_params([os("diff"), os("foo"), os("bar")].iter().cloned())
);
@@ -171,6 +179,7 @@ mod tests {
format: Format::Ed,
context_count: 3,
report_identical_files: false,
brief: false,
}),
parse_params([os("diff"), os("-e"), os("foo"), os("bar")].iter().cloned())
);
@@ -184,6 +193,7 @@ mod tests {
format: Format::Unified,
context_count: 54,
report_identical_files: false,
brief: false,
}),
parse_params(
[os("diff"), os("-u54"), os("foo"), os("bar")]
@@ -198,6 +208,7 @@ mod tests {
format: Format::Unified,
context_count: 54,
report_identical_files: false,
brief: false,
}),
parse_params(
[os("diff"), os("-U54"), os("foo"), os("bar")]
@@ -212,6 +223,7 @@ mod tests {
format: Format::Unified,
context_count: 54,
report_identical_files: false,
brief: false,
}),
parse_params(
[os("diff"), os("-U"), os("54"), os("foo"), os("bar")]
@@ -226,6 +238,7 @@ mod tests {
format: Format::Context,
context_count: 54,
report_identical_files: false,
brief: false,
}),
parse_params(
[os("diff"), os("-c54"), os("foo"), os("bar")]
@@ -243,6 +256,7 @@ mod tests {
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: false,
}),
parse_params([os("diff"), os("foo"), os("bar")].iter().cloned())
);
@@ -253,6 +267,7 @@ mod tests {
format: Format::Normal,
context_count: 3,
report_identical_files: true,
brief: false,
}),
parse_params([os("diff"), os("-s"), os("foo"), os("bar")].iter().cloned())
);
@@ -263,6 +278,7 @@ mod tests {
format: Format::Normal,
context_count: 3,
report_identical_files: true,
brief: false,
}),
parse_params(
[
@@ -277,6 +293,46 @@ mod tests {
);
}
#[test]
fn brief() {
assert_eq!(
Ok(Params {
from: os("foo"),
to: os("bar"),
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: false,
}),
parse_params([os("diff"), os("foo"), os("bar")].iter().cloned())
);
assert_eq!(
Ok(Params {
from: os("foo"),
to: os("bar"),
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: true,
}),
parse_params([os("diff"), os("-q"), os("foo"), os("bar")].iter().cloned())
);
assert_eq!(
Ok(Params {
from: os("foo"),
to: os("bar"),
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: true,
}),
parse_params(
[os("diff"), os("--brief"), os("foo"), os("bar"),]
.iter()
.cloned()
)
);
}
#[test]
fn double_dash() {
assert_eq!(
Ok(Params {
@@ -285,6 +341,7 @@ mod tests {
format: Format::Normal,
context_count: 3,
report_identical_files: false,
brief: false,
}),
parse_params([os("diff"), os("--"), os("-g"), os("-h")].iter().cloned())
);
+117 -13
View File
@@ -32,7 +32,12 @@ impl Mismatch {
}
// Produces a diff between the expected output and actual output.
fn make_diff(expected: &[u8], actual: &[u8], context_size: usize) -> Vec<Mismatch> {
fn make_diff(
expected: &[u8],
actual: &[u8],
context_size: usize,
stop_early: bool,
) -> Vec<Mismatch> {
let mut line_number_expected = 1;
let mut line_number_actual = 1;
let mut context_queue: VecDeque<&[u8]> = VecDeque::with_capacity(context_size);
@@ -180,6 +185,10 @@ fn make_diff(expected: &[u8], actual: &[u8], context_size: usize) -> Vec<Mismatc
line_number_actual += 1;
}
}
if stop_early && !results.is_empty() {
// Optimization: stop analyzing the files as soon as there are any differences
return results;
}
}
results.push(mismatch);
@@ -231,12 +240,16 @@ pub fn diff(
actual: &[u8],
actual_filename: &str,
context_size: usize,
stop_early: bool,
) -> Vec<u8> {
let mut output = format!("--- {expected_filename}\t\n+++ {actual_filename}\t\n").into_bytes();
let diff_results = make_diff(expected, actual, context_size);
let diff_results = make_diff(expected, actual, context_size, stop_early);
if diff_results.is_empty() {
return Vec::new();
};
}
if stop_early {
return output;
}
for result in diff_results {
let mut line_number_expected = result.line_number_expected;
let mut line_number_actual = result.line_number_actual;
@@ -434,8 +447,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alef", &bet, &format!("{target}/alef"), 2);
let diff = diff(
&alef,
"a/alef",
&bet,
&format!("{target}/alef"),
2,
false,
);
File::create(&format!("{target}/ab.diff"))
.unwrap()
.write_all(&diff)
@@ -542,8 +561,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alefn", &bet, &format!("{target}/alefn"), 2);
let diff = diff(
&alef,
"a/alefn",
&bet,
&format!("{target}/alefn"),
2,
false,
);
File::create(&format!("{target}/abn.diff"))
.unwrap()
.write_all(&diff)
@@ -630,8 +655,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alef_", &bet, &format!("{target}/alef_"), 2);
let diff = diff(
&alef,
"a/alef_",
&bet,
&format!("{target}/alef_"),
2,
false,
);
File::create(&format!("{target}/ab_.diff"))
.unwrap()
.write_all(&diff)
@@ -703,8 +734,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alefx", &bet, &format!("{target}/alefx"), 2);
let diff = diff(
&alef,
"a/alefx",
&bet,
&format!("{target}/alefx"),
2,
false,
);
File::create(&format!("{target}/abx.diff"))
.unwrap()
.write_all(&diff)
@@ -781,8 +818,14 @@ mod tests {
}
// This test diff is intentionally reversed.
// We want it to turn the alef into bet.
let diff =
diff(&alef, "a/alefr", &bet, &format!("{target}/alefr"), 2);
let diff = diff(
&alef,
"a/alefr",
&bet,
&format!("{target}/alefr"),
2,
false,
);
File::create(&format!("{target}/abr.diff"))
.unwrap()
.write_all(&diff)
@@ -810,4 +853,65 @@ mod tests {
}
}
}
#[test]
fn test_stop_early() {
let from_filename = "foo";
let from = vec!["a", "b", "c", ""].join("\n");
let to_filename = "bar";
let to = vec!["a", "d", "c", ""].join("\n");
let context_size: usize = 3;
let diff_full = diff(
from.as_bytes(),
from_filename,
to.as_bytes(),
to_filename,
context_size,
false,
);
let expected_full = vec![
"--- foo\t",
"+++ bar\t",
"@@ -1,3 +1,3 @@",
" a",
"-b",
"+d",
" c",
"",
]
.join("\n");
assert_eq!(diff_full, expected_full.as_bytes());
let diff_brief = diff(
from.as_bytes(),
from_filename,
to.as_bytes(),
to_filename,
context_size,
true,
);
let expected_brief = vec!["--- foo\t", "+++ bar\t", ""].join("\n");
assert_eq!(diff_brief, expected_brief.as_bytes());
let nodiff_full = diff(
from.as_bytes(),
from_filename,
from.as_bytes(),
to_filename,
context_size,
false,
);
assert!(nodiff_full.is_empty());
let nodiff_brief = diff(
from.as_bytes(),
from_filename,
from.as_bytes(),
to_filename,
context_size,
true,
);
assert!(nodiff_brief.is_empty());
}
}
+24
View File
@@ -123,6 +123,30 @@ fn differences() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}
#[test]
fn differences_brief() -> Result<(), Box<dyn std::error::Error>> {
let mut file1 = NamedTempFile::new()?;
file1.write_all("foo\n".as_bytes())?;
let mut file2 = NamedTempFile::new()?;
file2.write_all("bar\n".as_bytes())?;
for option in ["", "-u", "-c", "-e"] {
let mut cmd = Command::cargo_bin("diffutils")?;
if !option.is_empty() {
cmd.arg(option);
}
cmd.arg("-q").arg(file1.path()).arg(file2.path());
cmd.assert()
.code(predicate::eq(1))
.failure()
.stdout(predicate::eq(format!(
"Files {} and {} differ\n",
file1.path().to_string_lossy(),
file2.path().to_string_lossy()
)));
}
Ok(())
}
#[test]
fn missing_newline() -> Result<(), Box<dyn std::error::Error>> {
let mut file1 = NamedTempFile::new()?;