Skip to content

Commit

Permalink
Make error message consistent with GNU diff's implementation when fai…
Browse files Browse the repository at this point in the history
…ling to read input file(s)
  • Loading branch information
oSoMoN committed Apr 30, 2024
1 parent 61314ea commit c72564e
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 16 deletions.
33 changes: 29 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// files that was distributed with this source code.

use crate::params::{parse_params, Format};
use regex::Regex;
use std::env;
use std::ffi::OsString;
use std::fs;
Expand All @@ -18,6 +19,22 @@ mod params;
mod unified_diff;
mod utils;

fn report_failure_to_read_input_file(
executable: &OsString,
filepath: &OsString,
error: &std::io::Error,
) {
// std::io::Error's display trait outputs "{detail} (os error {code})"
// but we want only the {detail} (error string) part
let error_code_re = Regex::new(r"\ \(os\ error\ \d+\)$").unwrap();
eprintln!(
"{}: {}: {}",
executable.to_string_lossy(),
filepath.to_string_lossy(),
error_code_re.replace(error.to_string().as_str(), ""),
);
}

// Exit codes are documented at
// https://www.gnu.org/software/diffutils/manual/html_node/Invoking-diff.html.
// An exit status of 0 means no differences were found,
Expand Down Expand Up @@ -45,6 +62,7 @@ fn main() -> ExitCode {
maybe_report_identical_files();
return ExitCode::SUCCESS;
}

// read files
fn read_file_contents(filepath: &OsString) -> io::Result<Vec<u8>> {
if filepath == "-" {
Expand All @@ -54,20 +72,27 @@ fn main() -> ExitCode {
fs::read(filepath)
}
}
let mut io_error = false;
let from_content = match read_file_contents(&params.from) {
Ok(from_content) => from_content,
Err(e) => {
eprintln!("Failed to read from-file: {e}");
return ExitCode::from(2);
report_failure_to_read_input_file(&params.executable, &params.from, &e);
io_error = true;
vec![]
}
};
let to_content = match read_file_contents(&params.to) {
Ok(to_content) => to_content,
Err(e) => {
eprintln!("Failed to read to-file: {e}");
return ExitCode::from(2);
report_failure_to_read_input_file(&params.executable, &params.to, &e);
io_error = true;
vec![]
}
};
if io_error {
return ExitCode::from(2);
}

// run diff
let result: Vec<u8> = match params.format {
Format::Normal => normal_diff::diff(&from_content, &to_content, &params),
Expand Down
55 changes: 49 additions & 6 deletions src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub enum Format {

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Params {
pub executable: OsString,
pub from: OsString,
pub to: OsString,
pub format: Format,
Expand All @@ -27,6 +28,7 @@ pub struct Params {
impl Default for Params {
fn default() -> Self {
Self {
executable: OsString::default(),
from: OsString::default(),
to: OsString::default(),
format: Format::default(),
Expand All @@ -43,10 +45,13 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
let mut opts = opts.into_iter().peekable();
// parse CLI

let Some(exe) = opts.next() else {
let Some(executable) = opts.next() else {
return Err("Usage: <exe> <from> <to>".to_string());
};
let mut params = Params::default();
let mut params = Params {
executable,
..Default::default()
};
let mut from = None;
let mut to = None;
let mut format = None;
Expand All @@ -63,7 +68,10 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
} else if to.is_none() {
to = Some(param);
} else {
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
return Err(format!(
"Usage: {} <from> <to>",
params.executable.to_string_lossy()
));
}
continue;
}
Expand Down Expand Up @@ -155,22 +163,31 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
} else if to.is_none() {
to = Some(param);
} else {
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
return Err(format!(
"Usage: {} <from> <to>",
params.executable.to_string_lossy()
));
}
}
params.from = if let Some(from) = from {
from
} else if let Some(param) = opts.next() {
param
} else {
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
return Err(format!(
"Usage: {} <from> <to>",
params.executable.to_string_lossy()
));
};
params.to = if let Some(to) = to {
to
} else if let Some(param) = opts.next() {
param
} else {
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
return Err(format!(
"Usage: {} <from> <to>",
params.executable.to_string_lossy()
));
};

// diff DIRECTORY FILE => diff DIRECTORY/FILE FILE
Expand Down Expand Up @@ -301,6 +318,7 @@ mod tests {
fn basics() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -309,6 +327,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -325,6 +344,7 @@ mod tests {
for arg in ["-e", "--ed"] {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Ed,
Expand All @@ -342,6 +362,7 @@ mod tests {
params.extend(["foo", "bar"]);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Context,
Expand All @@ -362,6 +383,7 @@ mod tests {
params.extend(["foo", "bar"]);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Context,
Expand Down Expand Up @@ -399,6 +421,7 @@ mod tests {
params.extend(["foo", "bar"]);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Unified,
Expand All @@ -419,6 +442,7 @@ mod tests {
params.extend(["foo", "bar"]);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Unified,
Expand Down Expand Up @@ -452,6 +476,7 @@ mod tests {
fn context_count() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Unified,
Expand All @@ -466,6 +491,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Unified,
Expand All @@ -480,6 +506,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Unified,
Expand All @@ -494,6 +521,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
format: Format::Context,
Expand All @@ -511,6 +539,7 @@ mod tests {
fn report_identical_files() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -519,6 +548,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
report_identical_files: true,
Expand All @@ -528,6 +558,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
report_identical_files: true,
Expand All @@ -549,6 +580,7 @@ mod tests {
fn brief() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -557,6 +589,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
brief: true,
Expand All @@ -566,6 +599,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
brief: true,
Expand All @@ -582,6 +616,7 @@ mod tests {
fn expand_tabs() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -591,6 +626,7 @@ mod tests {
for option in ["-t", "--expand-tabs"] {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
expand_tabs: true,
Expand All @@ -608,6 +644,7 @@ mod tests {
fn tabsize() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
..Default::default()
Expand All @@ -616,6 +653,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
tabsize: 0,
Expand All @@ -629,6 +667,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("bar"),
tabsize: 42,
Expand Down Expand Up @@ -686,6 +725,7 @@ mod tests {
fn double_dash() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("-g"),
to: os("-h"),
..Default::default()
Expand All @@ -697,6 +737,7 @@ mod tests {
fn default_to_stdin() {
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("foo"),
to: os("-"),
..Default::default()
Expand All @@ -705,6 +746,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("-"),
to: os("bar"),
..Default::default()
Expand All @@ -713,6 +755,7 @@ mod tests {
);
assert_eq!(
Ok(Params {
executable: os("diff"),
from: os("-"),
to: os("-"),
..Default::default()
Expand Down
21 changes: 15 additions & 6 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,30 @@ fn cannot_read_files() -> Result<(), Box<dyn std::error::Error>> {
cmd.assert()
.code(predicate::eq(2))
.failure()
.stderr(predicate::str::starts_with("Failed to read from-file"));
.stderr(predicate::str::ends_with(format!(
": {}: No such file or directory\n",
&nopath.as_os_str().to_string_lossy()
)));

let mut cmd = Command::cargo_bin("diffutils")?;
cmd.arg(file.path()).arg(&nopath);
cmd.assert()
.code(predicate::eq(2))
.failure()
.stderr(predicate::str::starts_with("Failed to read to-file"));
.stderr(predicate::str::ends_with(format!(
": {}: No such file or directory\n",
&nopath.as_os_str().to_string_lossy()
)));

let mut cmd = Command::cargo_bin("diffutils")?;
cmd.arg(&nopath).arg(&nopath);
cmd.assert()
.code(predicate::eq(2))
.failure()
.stderr(predicate::str::starts_with("Failed to read from-file"));
cmd.assert().code(predicate::eq(2)).failure().stderr(
predicate::str::contains(format!(
": {}: No such file or directory\n",
&nopath.as_os_str().to_string_lossy()
))
.count(2),
);

Ok(())
}
Expand Down

0 comments on commit c72564e

Please sign in to comment.