Skip to content

Commit c72564e

Browse files
oSoMoNoSoMoN
committed
Make error message consistent with GNU diff's implementation when failing to read input file(s)
1 parent 61314ea commit c72564e

File tree

3 files changed

+93
-16
lines changed

3 files changed

+93
-16
lines changed

‎src/main.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// files that was distributed with this source code.
55

66
use crate::params::{parse_params, Format};
7+
use regex::Regex;
78
use std::env;
89
use std::ffi::OsString;
910
use std::fs;
@@ -18,6 +19,22 @@ mod params;
1819
mod unified_diff;
1920
mod utils;
2021

22+
fn report_failure_to_read_input_file(
23+
executable: &OsString,
24+
filepath: &OsString,
25+
error: &std::io::Error,
26+
) {
27+
// std::io::Error's display trait outputs "{detail} (os error {code})"
28+
// but we want only the {detail} (error string) part
29+
let error_code_re = Regex::new(r"\ \(os\ error\ \d+\)$").unwrap();
30+
eprintln!(
31+
"{}: {}: {}",
32+
executable.to_string_lossy(),
33+
filepath.to_string_lossy(),
34+
error_code_re.replace(error.to_string().as_str(), ""),
35+
);
36+
}
37+
2138
// Exit codes are documented at
2239
// https://www.gnu.org/software/diffutils/manual/html_node/Invoking-diff.html.
2340
// An exit status of 0 means no differences were found,
@@ -45,6 +62,7 @@ fn main() -> ExitCode {
4562
maybe_report_identical_files();
4663
return ExitCode::SUCCESS;
4764
}
65+
4866
// read files
4967
fn read_file_contents(filepath: &OsString) -> io::Result<Vec<u8>> {
5068
if filepath == "-" {
@@ -54,20 +72,27 @@ fn main() -> ExitCode {
5472
fs::read(filepath)
5573
}
5674
}
75+
let mut io_error = false;
5776
let from_content = match read_file_contents(&params.from) {
5877
Ok(from_content) => from_content,
5978
Err(e) => {
60-
eprintln!("Failed to read from-file: {e}");
61-
return ExitCode::from(2);
79+
report_failure_to_read_input_file(&params.executable, &params.from, &e);
80+
io_error = true;
81+
vec![]
6282
}
6383
};
6484
let to_content = match read_file_contents(&params.to) {
6585
Ok(to_content) => to_content,
6686
Err(e) => {
67-
eprintln!("Failed to read to-file: {e}");
68-
return ExitCode::from(2);
87+
report_failure_to_read_input_file(&params.executable, &params.to, &e);
88+
io_error = true;
89+
vec![]
6990
}
7091
};
92+
if io_error {
93+
return ExitCode::from(2);
94+
}
95+
7196
// run diff
7297
let result: Vec<u8> = match params.format {
7398
Format::Normal => normal_diff::diff(&from_content, &to_content, &params),

‎src/params.rs

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub enum Format {
1414

1515
#[derive(Clone, Debug, Eq, PartialEq)]
1616
pub struct Params {
17+
pub executable: OsString,
1718
pub from: OsString,
1819
pub to: OsString,
1920
pub format: Format,
@@ -27,6 +28,7 @@ pub struct Params {
2728
impl Default for Params {
2829
fn default() -> Self {
2930
Self {
31+
executable: OsString::default(),
3032
from: OsString::default(),
3133
to: OsString::default(),
3234
format: Format::default(),
@@ -43,10 +45,13 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
4345
let mut opts = opts.into_iter().peekable();
4446
// parse CLI
4547

46-
let Some(exe) = opts.next() else {
48+
let Some(executable) = opts.next() else {
4749
return Err("Usage: <exe> <from> <to>".to_string());
4850
};
49-
let mut params = Params::default();
51+
let mut params = Params {
52+
executable,
53+
..Default::default()
54+
};
5055
let mut from = None;
5156
let mut to = None;
5257
let mut format = None;
@@ -63,7 +68,10 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
6368
} else if to.is_none() {
6469
to = Some(param);
6570
} else {
66-
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
71+
return Err(format!(
72+
"Usage: {} <from> <to>",
73+
params.executable.to_string_lossy()
74+
));
6775
}
6876
continue;
6977
}
@@ -155,22 +163,31 @@ pub fn parse_params<I: IntoIterator<Item = OsString>>(opts: I) -> Result<Params,
155163
} else if to.is_none() {
156164
to = Some(param);
157165
} else {
158-
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
166+
return Err(format!(
167+
"Usage: {} <from> <to>",
168+
params.executable.to_string_lossy()
169+
));
159170
}
160171
}
161172
params.from = if let Some(from) = from {
162173
from
163174
} else if let Some(param) = opts.next() {
164175
param
165176
} else {
166-
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
177+
return Err(format!(
178+
"Usage: {} <from> <to>",
179+
params.executable.to_string_lossy()
180+
));
167181
};
168182
params.to = if let Some(to) = to {
169183
to
170184
} else if let Some(param) = opts.next() {
171185
param
172186
} else {
173-
return Err(format!("Usage: {} <from> <to>", exe.to_string_lossy()));
187+
return Err(format!(
188+
"Usage: {} <from> <to>",
189+
params.executable.to_string_lossy()
190+
));
174191
};
175192

176193
// diff DIRECTORY FILE => diff DIRECTORY/FILE FILE
@@ -301,6 +318,7 @@ mod tests {
301318
fn basics() {
302319
assert_eq!(
303320
Ok(Params {
321+
executable: os("diff"),
304322
from: os("foo"),
305323
to: os("bar"),
306324
..Default::default()
@@ -309,6 +327,7 @@ mod tests {
309327
);
310328
assert_eq!(
311329
Ok(Params {
330+
executable: os("diff"),
312331
from: os("foo"),
313332
to: os("bar"),
314333
..Default::default()
@@ -325,6 +344,7 @@ mod tests {
325344
for arg in ["-e", "--ed"] {
326345
assert_eq!(
327346
Ok(Params {
347+
executable: os("diff"),
328348
from: os("foo"),
329349
to: os("bar"),
330350
format: Format::Ed,
@@ -342,6 +362,7 @@ mod tests {
342362
params.extend(["foo", "bar"]);
343363
assert_eq!(
344364
Ok(Params {
365+
executable: os("diff"),
345366
from: os("foo"),
346367
to: os("bar"),
347368
format: Format::Context,
@@ -362,6 +383,7 @@ mod tests {
362383
params.extend(["foo", "bar"]);
363384
assert_eq!(
364385
Ok(Params {
386+
executable: os("diff"),
365387
from: os("foo"),
366388
to: os("bar"),
367389
format: Format::Context,
@@ -399,6 +421,7 @@ mod tests {
399421
params.extend(["foo", "bar"]);
400422
assert_eq!(
401423
Ok(Params {
424+
executable: os("diff"),
402425
from: os("foo"),
403426
to: os("bar"),
404427
format: Format::Unified,
@@ -419,6 +442,7 @@ mod tests {
419442
params.extend(["foo", "bar"]);
420443
assert_eq!(
421444
Ok(Params {
445+
executable: os("diff"),
422446
from: os("foo"),
423447
to: os("bar"),
424448
format: Format::Unified,
@@ -452,6 +476,7 @@ mod tests {
452476
fn context_count() {
453477
assert_eq!(
454478
Ok(Params {
479+
executable: os("diff"),
455480
from: os("foo"),
456481
to: os("bar"),
457482
format: Format::Unified,
@@ -466,6 +491,7 @@ mod tests {
466491
);
467492
assert_eq!(
468493
Ok(Params {
494+
executable: os("diff"),
469495
from: os("foo"),
470496
to: os("bar"),
471497
format: Format::Unified,
@@ -480,6 +506,7 @@ mod tests {
480506
);
481507
assert_eq!(
482508
Ok(Params {
509+
executable: os("diff"),
483510
from: os("foo"),
484511
to: os("bar"),
485512
format: Format::Unified,
@@ -494,6 +521,7 @@ mod tests {
494521
);
495522
assert_eq!(
496523
Ok(Params {
524+
executable: os("diff"),
497525
from: os("foo"),
498526
to: os("bar"),
499527
format: Format::Context,
@@ -511,6 +539,7 @@ mod tests {
511539
fn report_identical_files() {
512540
assert_eq!(
513541
Ok(Params {
542+
executable: os("diff"),
514543
from: os("foo"),
515544
to: os("bar"),
516545
..Default::default()
@@ -519,6 +548,7 @@ mod tests {
519548
);
520549
assert_eq!(
521550
Ok(Params {
551+
executable: os("diff"),
522552
from: os("foo"),
523553
to: os("bar"),
524554
report_identical_files: true,
@@ -528,6 +558,7 @@ mod tests {
528558
);
529559
assert_eq!(
530560
Ok(Params {
561+
executable: os("diff"),
531562
from: os("foo"),
532563
to: os("bar"),
533564
report_identical_files: true,
@@ -549,6 +580,7 @@ mod tests {
549580
fn brief() {
550581
assert_eq!(
551582
Ok(Params {
583+
executable: os("diff"),
552584
from: os("foo"),
553585
to: os("bar"),
554586
..Default::default()
@@ -557,6 +589,7 @@ mod tests {
557589
);
558590
assert_eq!(
559591
Ok(Params {
592+
executable: os("diff"),
560593
from: os("foo"),
561594
to: os("bar"),
562595
brief: true,
@@ -566,6 +599,7 @@ mod tests {
566599
);
567600
assert_eq!(
568601
Ok(Params {
602+
executable: os("diff"),
569603
from: os("foo"),
570604
to: os("bar"),
571605
brief: true,
@@ -582,6 +616,7 @@ mod tests {
582616
fn expand_tabs() {
583617
assert_eq!(
584618
Ok(Params {
619+
executable: os("diff"),
585620
from: os("foo"),
586621
to: os("bar"),
587622
..Default::default()
@@ -591,6 +626,7 @@ mod tests {
591626
for option in ["-t", "--expand-tabs"] {
592627
assert_eq!(
593628
Ok(Params {
629+
executable: os("diff"),
594630
from: os("foo"),
595631
to: os("bar"),
596632
expand_tabs: true,
@@ -608,6 +644,7 @@ mod tests {
608644
fn tabsize() {
609645
assert_eq!(
610646
Ok(Params {
647+
executable: os("diff"),
611648
from: os("foo"),
612649
to: os("bar"),
613650
..Default::default()
@@ -616,6 +653,7 @@ mod tests {
616653
);
617654
assert_eq!(
618655
Ok(Params {
656+
executable: os("diff"),
619657
from: os("foo"),
620658
to: os("bar"),
621659
tabsize: 0,
@@ -629,6 +667,7 @@ mod tests {
629667
);
630668
assert_eq!(
631669
Ok(Params {
670+
executable: os("diff"),
632671
from: os("foo"),
633672
to: os("bar"),
634673
tabsize: 42,
@@ -686,6 +725,7 @@ mod tests {
686725
fn double_dash() {
687726
assert_eq!(
688727
Ok(Params {
728+
executable: os("diff"),
689729
from: os("-g"),
690730
to: os("-h"),
691731
..Default::default()
@@ -697,6 +737,7 @@ mod tests {
697737
fn default_to_stdin() {
698738
assert_eq!(
699739
Ok(Params {
740+
executable: os("diff"),
700741
from: os("foo"),
701742
to: os("-"),
702743
..Default::default()
@@ -705,6 +746,7 @@ mod tests {
705746
);
706747
assert_eq!(
707748
Ok(Params {
749+
executable: os("diff"),
708750
from: os("-"),
709751
to: os("bar"),
710752
..Default::default()
@@ -713,6 +755,7 @@ mod tests {
713755
);
714756
assert_eq!(
715757
Ok(Params {
758+
executable: os("diff"),
716759
from: os("-"),
717760
to: os("-"),
718761
..Default::default()

‎tests/integration.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,30 @@ fn cannot_read_files() -> Result<(), Box<dyn std::error::Error>> {
3636
cmd.assert()
3737
.code(predicate::eq(2))
3838
.failure()
39-
.stderr(predicate::str::starts_with("Failed to read from-file"));
39+
.stderr(predicate::str::ends_with(format!(
40+
": {}: No such file or directory\n",
41+
&nopath.as_os_str().to_string_lossy()
42+
)));
4043

4144
let mut cmd = Command::cargo_bin("diffutils")?;
4245
cmd.arg(file.path()).arg(&nopath);
4346
cmd.assert()
4447
.code(predicate::eq(2))
4548
.failure()
46-
.stderr(predicate::str::starts_with("Failed to read to-file"));
49+
.stderr(predicate::str::ends_with(format!(
50+
": {}: No such file or directory\n",
51+
&nopath.as_os_str().to_string_lossy()
52+
)));
4753

4854
let mut cmd = Command::cargo_bin("diffutils")?;
4955
cmd.arg(&nopath).arg(&nopath);
50-
cmd.assert()
51-
.code(predicate::eq(2))
52-
.failure()
53-
.stderr(predicate::str::starts_with("Failed to read from-file"));
56+
cmd.assert().code(predicate::eq(2)).failure().stderr(
57+
predicate::str::contains(format!(
58+
": {}: No such file or directory\n",
59+
&nopath.as_os_str().to_string_lossy()
60+
))
61+
.count(2),
62+
);
5463

5564
Ok(())
5665
}

0 commit comments

Comments
 (0)