code cleanup

* ArgParser: removed default option, which does not exist in GNU diffutils
* fix test 'cannot_read_files' (same as PR #188)
* merge test from PR #159

Author: Gunter Schmidt

src/arg_parser.rs

@@ -25,13 +25,11 @@ pub const OPT_HELP: AppOption = AppOption {
     long_name: "help",
     short: None,
     has_arg: false,
-    arg_default: None,
 };
 pub const OPT_VERSION: AppOption = AppOption {
     long_name: "version",
     short: Some('v'),
     has_arg: false,
-    arg_default: None,
 };
 
 /// This contains the args/options the app allows. They must be all of const value.
@@ -41,26 +39,9 @@ pub struct AppOption {
     pub long_name: &'static str,
     pub short: Option<char>,
     pub has_arg: bool,
-    pub arg_default: Option<&'static str>,
+    // pub arg_default: Option<&'static str>,
 }
 
-// #[derive(Debug)]
-// pub struct AppOptions(&'static [AppOption]);
-//
-// impl AppOptions {
-//     fn identify_options_from_partial_text(&self, opt: &str) -> Vec<&'static AppOption> {
-//         let l = opt.len();
-//         let v: Vec<&'static AppOption> = self
-//             .0
-//             .iter()
-//             .filter(|&it| it.long.len() >= l && &it.long[0..l] == opt)
-//             // .copied()
-//             .collect();
-//
-//         v
-//     }
-// }
-
 #[derive(Debug, Clone, PartialEq)]
 pub struct ParsedOption {
     pub app_option: &'static AppOption,
@@ -106,11 +87,7 @@ impl ParsedOption {
                     }
                 }
                 if self.arg_for_option.is_none() {
-                    if let Some(default) = self.app_option.arg_default {
-                        self.arg_for_option = Some(default.to_string())
-                    } else {
-                        return Err(ArgParserError::ArgForOptionMissing(self.clone()));
-                    }
+                    return Err(ArgParserError::ArgForOptionMissing(self.clone()));
                 }
             }
         } else {
@@ -145,7 +122,6 @@ impl Default for ParsedOption {
                 long_name: "dummy",
                 short: None,
                 has_arg: false,
-                arg_default: None,
             },
             arg_for_option: None,
             name_type_used: OptionNameTypeUsed::LongName,
@@ -250,9 +226,6 @@ impl Display for ArgParserError {
             ArgParserError::NoExecutable => {
                 write!(f, "Expected utility name as second argument, got nothing.")
             }
-            // ParamsGenParseError::IgnoreInitialDouble( op3, ig) => {
-            //     write_err(f,  &format!("option '--ignore-initial' ('-i') is set to {ig} but also values ares passed as operand '{op3}'"))
-            // }
             ArgParserError::InvalidOption(opt) => {
                 write!(f, "{}", &format!("invalid option '{opt}'"))
             }
@@ -493,6 +466,10 @@ impl ArgParser {
         Ok(arg_parser)
     }
 
+    pub fn add_copyright(text: &str) -> String {
+        format!("{text}\n{TEXT_COPYRIGHT}")
+    }
+
     pub fn identify_options_from_partial_text(
         app_options: &'static [AppOption],
         opt: &str,

src/diff.rs

@@ -39,10 +39,10 @@ pub fn main(opts: Peekable<ArgsOs>) -> ExitCode {
 
     let (from_content, to_content) = match utils::read_both_files(&params.from, &params.to) {
         Ok(contents) => contents,
-        Err((filepath, error)) => {
+        Err(e) => {
             eprintln!(
                 "{}",
-                utils::format_failure_to_read_input_file(&params.executable, &filepath, &error)
+                utils::format_failure_to_read_input_files(&params.executable, &e)
             );
             return ExitCode::from(2);
         }

src/sdiff.rs

@@ -91,12 +91,9 @@ impl Display for SdiffError {
 pub fn sdiff(params: &ParamsSdiff) -> Result<SdiffOk, SdiffError> {
     let (from_content, to_content) = match utils::read_both_files(&params.from, &params.to) {
         Ok(contents) => contents,
-        Err((filepath, error)) => {
-            let msg = utils::format_failure_to_read_input_file(
-                &params.util.executable(),
-                &filepath,
-                &error,
-            );
+        // Err((filepath, error)) => {
+        Err(errors) => {
+            let msg = utils::format_failure_to_read_input_files(&params.util.executable(), &errors);
             return Err(SdiffError::ReadFileError(msg));
         }
     };

src/sdiff/params_sdiff.rs

@@ -75,7 +75,7 @@ impl ParamsSdiff {
             match *parsed_option.app_option {
                 OPT_DIFF_PROGRAM => params.diff_program = parsed_option.arg_for_option.clone(),
                 OPT_EXPAND_TABS => params.expand_tabs = true,
-                OPT_HELP => return Ok(ParamsSdiffOk::Info(ParamsSdiffInfo::Help)),
+                OPT_HELP => return Ok(ParamsSdiffOk::Info(ArgParser::add_copyright(TEXT_HELP))),
                 OPT_IGNORE_ALL_SPACE => params.ignore_all_space = true,
                 OPT_IGNORE_BLANK_LINES => params.ignore_blank_lines = true,
                 OPT_IGNORE_CASE => params.ignore_case = true,
@@ -95,7 +95,7 @@ impl ParamsSdiff {
                     params.set_tabsize(parsed_option)?;
                 }
                 OPT_TEXT => params.text = true,
-                OPT_VERSION => return Ok(ParamsSdiffOk::Info(ParamsSdiffInfo::Version)),
+                OPT_VERSION => return Ok(ParamsSdiffOk::Info(TEXT_VERSION.to_string())),
                 OPT_WIDTH => {
                     params.set_width(parsed_option)?;
                 }

src/sdiff/params_sdiff_def.rs

@@ -5,8 +5,7 @@ use const_format::concatcp;
 
 use crate::{
     arg_parser::{
-        AppOption, ArgParserError, ParsedOption, OPT_HELP, OPT_VERSION, TEXT_COPYRIGHT,
-        TEXT_HELP_FOOTER,
+        AppOption, ArgParserError, ParsedOption, OPT_HELP, OPT_VERSION, TEXT_HELP_FOOTER,
     },
     sdiff::{params_sdiff::ParamsSdiff, EXE_NAME},
     // sdiff::{Bytes, IgnInit, EXE_NAME},
@@ -19,109 +18,91 @@ pub(super) const OPT_DIFF_PROGRAM: AppOption = AppOption {
     long_name: "diff-program",
     short: None,
     has_arg: true,
-    arg_default: None,
 };
 pub(super) const OPT_EXPAND_TABS: AppOption = AppOption {
     long_name: "expand-tabs",
     short: Some('t'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_ALL_SPACE: AppOption = AppOption {
     long_name: "ignore-all-space",
     short: Some('W'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_BLANK_LINES: AppOption = AppOption {
     long_name: "ignore-blank-lines",
     short: Some('B'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_CASE: AppOption = AppOption {
     long_name: "ignore-case",
     short: Some('i'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_MATCHING_LINES: AppOption = AppOption {
     long_name: "ignore-matching-lines",
     short: Some('I'),
     has_arg: true,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_SPACE_CHANGE: AppOption = AppOption {
     long_name: "ignore-space-change",
     short: Some('b'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_TAB_EXPANSION: AppOption = AppOption {
     long_name: "ignore-tab-expansion",
     short: Some('E'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_IGNORE_TRAILING_SPACE: AppOption = AppOption {
     long_name: "ignore-trailing-space",
     short: Some('Z'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_LEFT_COLUMN: AppOption = AppOption {
     long_name: "left-column",
     short: Some('l'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_MINIMAL: AppOption = AppOption {
     long_name: "minimal",
     short: Some('d'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_OUTPUT: AppOption = AppOption {
     long_name: "output",
     short: Some('o'),
     has_arg: true,
-    arg_default: None,
 };
 pub(super) const OPT_SPEED_LARGE_FILES: AppOption = AppOption {
     long_name: "speed-large-files",
     short: Some('H'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_STRIP_TRAILING_CR: AppOption = AppOption {
     long_name: "strip-trailing-cr",
     short: None,
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_SUPPRESS_COMMON_LINES: AppOption = AppOption {
     long_name: "suppress-common-lines",
     short: Some('s'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_TABSIZE: AppOption = AppOption {
     long_name: "tabsize",
     short: None,
     has_arg: true,
-    arg_default: Some("8"),
 };
 pub(super) const OPT_TEXT: AppOption = AppOption {
     long_name: "text",
     short: Some('a'),
     has_arg: false,
-    arg_default: None,
 };
 pub(super) const OPT_WIDTH: AppOption = AppOption {
     long_name: "width",
     short: Some('w'),
     has_arg: true,
-    arg_default: None,
 };
 
 // Array for ParamsGen
@@ -200,31 +181,10 @@ pub const TEXT_VERSION: &str = concat!("sdiff (Rust DiffUtils) ", env!("CARGO_PK
 /// Error will be returned as [ParamsSdiffError] in the function Result.
 #[derive(Debug, PartialEq)]
 pub enum ParamsSdiffOk {
-    Info(ParamsSdiffInfo),
+    Info(String),
     ParamsSdiff(ParamsSdiff),
 }
 
-/// Static texts for '--help' and '--version'.
-///
-/// The parser returns these enums to the caller, allowing to identify this as information,
-/// so the program exit code is SUCCESS(0).
-#[derive(Debug, PartialEq)]
-pub enum ParamsSdiffInfo {
-    Help,
-    Version,
-}
-
-impl Display for ParamsSdiffInfo {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let info = match self {
-            ParamsSdiffInfo::Help => TEXT_HELP,
-            ParamsSdiffInfo::Version => &format!("{TEXT_VERSION}\n{TEXT_COPYRIGHT}"),
-        };
-
-        write!(f, "{}", info)
-    }
-}
-
 /// Contains all parser errors and their text messages.
 /// This allows centralized maintenance.
 #[derive(Debug, PartialEq)]

src/utils.rs

@@ -88,6 +88,28 @@ pub fn format_failure_to_read_input_file(
     )
 }
 
+/// Formats the error messages of both files.
+pub fn format_failure_to_read_input_files(
+    executable: &OsString,
+    errors: &[(OsString, Error)],
+) -> String {
+    let mut msg = format_failure_to_read_input_file(
+        executable,
+        &errors[0].0, // filepath,
+        &errors[0].1, // &error,
+    );
+    if errors.len() > 1 {
+        msg.push('\n');
+        msg.push_str(&format_failure_to_read_input_file(
+            executable,
+            &errors[1].0, // filepath,
+            &errors[1].1, // &error,
+        ));
+    }
+
+    msg
+}
+
 pub fn read_file_contents(filepath: &OsString) -> io::Result<Vec<u8>> {
     if filepath == "-" {
         let mut content = Vec::new();
@@ -97,13 +119,30 @@ pub fn read_file_contents(filepath: &OsString) -> io::Result<Vec<u8>> {
     }
 }
 
-pub fn read_both_files(
-    from: &OsString,
-    to: &OsString,
-) -> Result<(Vec<u8>, Vec<u8>), (OsString, Error)> {
-    let from_content = read_file_contents(from).map_err(|e| (from.clone(), e))?;
-    let to_content = read_file_contents(to).map_err(|e| (to.clone(), e))?;
-    Ok((from_content, to_content))
+pub type ResultReadBothFiles = Result<(Vec<u8>, Vec<u8>), Vec<(OsString, Error)>>;
+/// Reads both files and returns the files or a list of errors, as both files can produce a separate error.
+pub fn read_both_files(from: &OsString, to: &OsString) -> ResultReadBothFiles {
+    let mut read_errors = Vec::new();
+    let from_content = match read_file_contents(from).map_err(|e| (from.clone(), e)) {
+        Ok(r) => r,
+        Err(e) => {
+            read_errors.push(e);
+            Vec::new()
+        }
+    };
+    let to_content = match read_file_contents(to).map_err(|e| (to.clone(), e)) {
+        Ok(r) => r,
+        Err(e) => {
+            read_errors.push(e);
+            Vec::new()
+        }
+    };
+
+    if read_errors.is_empty() {
+        Ok((from_content, to_content))
+    } else {
+        Err(read_errors)
+    }
 }
 
 #[cfg(test)]