Repository: yshavit/mdq (rust)

Issue:
Feature Request: Add parameter for file Input
Hello,

Many command-line tools that work with files typically provide a `-f` parameter to read files directly from disk instead of stdin. Multiple `-f` options may be used.

Would it be possible to add such a parameter to simplify usage for end users?

Thank you...

Pull request description:
read extra positional args as files
Treat the first positional arg as a selector (as before), but any subsequent ones as files to read. This resolves #275.

From the new docs:

> An optional list of Markdown files to parse, by path. If not provided, standard input will be used.
>
> If these are provided, mdq will act as if they were all concatenated into a single file. For example, if you
> use --link-pos=doc, the link definitions for all input files will be at the very end of the output.
>
> A path of "-" represents standard input.
>
> If these are provided, standard input will not be used unless one of the arguments is "-". Files will be
> processed in the order you provide them. If you provide the same file twice, mdq will process it twice, unless
> that file is "-"; all but the first "-" paths are ignored.

Now that I have the new `OsFacade`, there's an opportunity to have it include the output as well, such that we can remove the separate `run_in_memory` and `run_stdio` implementations. I'll leave that for #213, though.

Patch:
diff --git a/build.rs b/build.rs
index 4f42297..277e598 100644
--- a/build.rs
+++ b/build.rs
@@ -46,6 +46,18 @@ fn generate_integ_test_cases(out_dir: &String) -> Result<(), String> {
                 }
             });
 
+            out.write("const FILES: [(&str, &str); ");
+            out.write(&format!("{}", &spec_file_parsed.given.get_files_count()));
+            out.write("] = [");
+            if let Some(files) = &spec_file_parsed.given.files {
+                out.with_indent(|out| {
+                    for (file_name, file_content) in files {
+                        out.writeln(&format!("({:?}, {:?}),", file_name, file_content));
+                    }
+                });
+            }
+            out.writeln("];");
+
             let mut found_chained_case = false;
             for case in spec_file_parsed.get_cases() {
                 found_chained_case |= case.case_name.eq("chained");
@@ -124,6 +136,16 @@ struct TestSpecFile {
 #[derive(Deserialize)]
 struct TestGiven {
     md: String,
+    files: Option<HashMap<String, String>>,
+}
+
+impl TestGiven {
+    fn get_files_count(&self) -> usize {
+        match &self.files {
+            None => 0,
+            Some(files) => files.len(),
+        }
+    }
 }
 
 #[derive(Deserialize)]
@@ -232,7 +254,8 @@ impl Case {
                 out.write("expect_success: ")
                     .write(&self.expect_success.to_string())
                     .writeln(",");
-                out.write("md: MD,");
+                out.writeln("md: MD,");
+                out.write("files: &FILES,");
             });
             out.write("}.check();");
         });
diff --git a/src/cli.rs b/src/cli.rs
index 4e45a7d..03daf6d 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -79,6 +79,19 @@ pub struct Cli {
     // See: tree.rs > Lookups::unknown_markdown.
     #[arg(long, hide = true)]
     pub(crate) allow_unknown_markdown: bool,
+
+    /// An optional list of Markdown files to parse, by path. If not provided, standard input will be used.
+    ///
+    /// If these are provided, mdq will act as if they were all concatenated into a single file. For example, if you
+    /// use --link-pos=doc, the link definitions for all input files will be at the very end of the output.
+    ///
+    /// A path of "-" represents standard input.
+    ///
+    /// If these are provided, standard input will not be used unless one of the arguments is "-". Files will be
+    /// processed in the order you provide them. If you provide the same file twice, mdq will process it twice, unless
+    /// that file is "-"; all but the first "-" paths are ignored.
+    #[arg()]
+    pub(crate) markdown_file_paths: Vec<String>,
 }
 
 impl Cli {
@@ -206,17 +219,28 @@ mod tests {
         Cli::command().debug_assert();
     }
 
+    #[test]
+    fn no_args() {
+        let result = Cli::try_parse_from(["mdq"]);
+        unwrap!(result, Ok(cli));
+        assert_eq!(cli.selector_string().as_str(), "");
+        assert!(cli.markdown_file_paths.is_empty());
+    }
+
     #[test]
     fn standard_selectors() {
         let result = Cli::try_parse_from(["mdq", "# hello"]);
         unwrap!(result, Ok(cli));
         assert_eq!(cli.selector_string().as_str(), "# hello");
+        assert!(cli.markdown_file_paths.is_empty());
     }
 
     #[test]
-    fn two_standard_selectors() {
-        let result = Cli::try_parse_from(["mdq", "# hello", "# world"]);
-        check_err(&result, "unexpected argument '# world' found");
+    fn selector_and_file() {
+        let result = Cli::try_parse_from(["mdq", "# hello", "file.txt"]);
+        unwrap!(result, Ok(cli));
+        assert_eq!(cli.selector_string().as_str(), "# hello");
+        assert_eq!(cli.markdown_file_paths, ["file.txt"]);
     }
 
     #[test]
diff --git a/src/lib.rs b/src/lib.rs
index 9d554f0..2707260 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -12,8 +12,8 @@ use pest::error::ErrorVariant;
 use pest::Span;
 use std::borrow::Cow;
 use std::fmt::{Display, Formatter};
-use std::io;
-use std::io::{stdin, Read, Write};
+use std::io::Write;
+use std::{env, io};
 
 pub mod cli;
 mod fmt_md;
@@ -40,6 +40,28 @@ mod words_buffer;
 pub enum Error {
     QueryParse { query_string: String, error: ParseError },
     MarkdownParse(InvalidMd),
+    FileReadError(File, io::Error),
+}
+
+#[derive(Debug)]
+pub enum File {
+    Stdin,
+    Path(String),
+}
+
+impl Error {
+    pub fn from_io_error(error: io::Error, file: File) -> Self {
+        Error::FileReadError(file, error)
+    }
+}
+
+impl Display for File {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            File::Stdin => f.write_str("stdin"),
+            File::Path(file) => write!(f, "file {file:?}"),
+        }
+    }
 }
 
 impl Display for Error {
@@ -66,36 +88,82 @@ impl Display for Error {
                 writeln!(f, "Markdown parse error:")?;
                 writeln!(f, "{err}")
             }
+            Error::FileReadError(file, err) => {
+                if env::var("MDQ_PORTABLE_ERRORS").unwrap_or(String::new()).is_empty() {
+                    writeln!(f, "{err} while reading {file}")
+                } else {
+                    writeln!(f, "{} while reading {file}", err.kind())
+                }
+            }
+        }
+    }
+}
+
+pub trait OsFacade {
+    fn read_stdin(&self) -> io::Result<String>;
+    fn read_file(&self, path: &str) -> io::Result<String>;
+
+    fn read_all(&self, cli: &Cli) -> Result<String, Error> {
+        if cli.markdown_file_paths.is_empty() {
+            return self.read_stdin().map_err(|err| Error::from_io_error(err, File::Stdin));
         }
+        let mut contents = String::new();
+        let mut have_read_stdin = false;
+        for path in &cli.markdown_file_paths {
+            if path == "-" {
+                if !have_read_stdin {
+                    contents.push_str(
+                        &self
+                            .read_stdin()
+                            .map_err(|err| Error::from_io_error(err, File::Stdin))?,
+                    );
+                    have_read_stdin = true
+                }
+            } else {
+                let path_contents = self
+                    .read_file(path)
+                    .map_err(|err| Error::from_io_error(err, File::Path(path.to_string())))?;
+                contents.push_str(&path_contents);
+            }
+            contents.push('\n');
+        }
+        Ok(contents)
     }
 }
 
-pub fn run_in_memory(cli: &Cli, contents: &str) -> Result<(bool, String), Error> {
+pub fn run_in_memory(cli: &Cli, os: impl OsFacade) -> Result<(bool, String), Error> {
+    let contents_str = os.read_all(&cli)?;
     let mut out = Vec::with_capacity(256); // just a guess
 
-    let result = run(&cli, contents.to_string(), || &mut out)?;
+    let result = run(&cli, contents_str, || &mut out)?;
     let out_str = String::from_utf8(out).unwrap_or_else(|err| String::from_utf8_lossy(err.as_bytes()).into_owned());
     Ok((result, out_str))
 }
 
-pub fn run_stdio(cli: &Cli) -> bool {
+pub fn run_stdio(cli: &Cli, os: impl OsFacade) -> bool {
     if !cli.extra_validation() {
         return false;
     }
-    let mut contents = String::new();
-    stdin().read_to_string(&mut contents).expect("invalid input (not utf8)");
+    let contents = match os.read_all(&cli) {
+        Ok(s) => s,
+        Err(err) => {
+            eprint!("{err}");
+            return false;
+        }
+    };
+
     run(&cli, contents, || io::stdout().lock()).unwrap_or_else(|err| {
         eprint!("{err}");
         false
     })
 }
 
-fn run<W, F>(cli: &Cli, contents: String, get_out: F) -> Result<bool, Error>
+fn run<W, F>(cli: &Cli, stdin_content: String, get_out: F) -> Result<bool, Error>
 where
     F: FnOnce() -> W,
     W: Write,
 {
-    let ast = markdown::to_mdast(&contents, &markdown::ParseOptions::gfm()).unwrap();
+    let ast = markdown::to_mdast(&stdin_content, &markdown::ParseOptions::gfm()).unwrap();
     let read_options = ReadOptions {
         validate_no_conflicting_links: false,
         allow_unknown_markdown: cli.allow_unknown_markdown,
diff --git a/src/main.rs b/src/main.rs
index 67e9040..f7ba8d4 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,12 +1,28 @@
 use clap::Parser;
 use mdq::cli::Cli;
-use mdq::run_stdio;
+use mdq::{run_stdio, OsFacade};
+use std::io;
+use std::io::{stdin, Read};
 use std::process::ExitCode;
 
+struct RealOs;
+
+impl OsFacade for RealOs {
+    fn read_stdin(&self) -> io::Result<String> {
+        let mut contents = String::new();
+        stdin().read_to_string(&mut contents)?;
+        Ok(contents)
+    }
+
+    fn read_file(&self, path: &str) -> io::Result<String> {
+        std::fs::read_to_string(path)
+    }
+}
+
 fn main() -> ExitCode {
     let cli = Cli::parse();
 
-    if run_stdio(&cli) {
+    if run_stdio(&cli, RealOs) {
         ExitCode::SUCCESS
     } else {
         ExitCode::FAILURE


Test patch:
diff --git a/scripts/system_test b/scripts/system_test
index 31d7ae9..3dd2b65 100755
--- a/scripts/system_test
+++ b/scripts/system_test
@@ -63,6 +63,11 @@ function run_test_spec() {
     local stdin
     pushd "$(mktemp -d)"
 
+    while read -r md_test_file; do
+      local write_to="$md_test_file"
+      jq -r '.given.files[$name]' <<<"$spec" --arg name "$md_test_file" > "$write_to"
+    done <<<"$(jq -r '.given.files | keys | .[]' <<<"$spec")"
+
     if jq -e '.expect.ignore' <<<"$spec" &>/dev/null ; then
       msg warning "$full_name" 'skipping test case'
       return 0
@@ -81,7 +86,7 @@ function run_test_spec() {
 
     local actual_success=true
     set -x
-    "$mdq" <<<"$stdin" "${cli_args[@]}" >actual_out.txt 2>actual_err.txt || actual_success=false
+    MDQ_PORTABLE_ERRORS=1 "$mdq" <<<"$stdin" "${cli_args[@]}" >actual_out.txt 2>actual_err.txt || actual_success=false
     set +x
 
     if [[ "$output_json" == true ]]; then
diff --git a/tests/integ_test.rs b/tests/integ_test.rs
index 263b731..827ce90 100644
--- a/tests/integ_test.rs
+++ b/tests/integ_test.rs
@@ -1,4 +1,7 @@
 use clap::Parser;
+use std::ffi::OsString;
+use std::io::ErrorKind;
+use std::{env, io};
 
 #[derive(Debug)]
 struct Case<const N: usize> {
@@ -7,9 +10,25 @@ struct Case<const N: usize> {
     expect_error: &'static str,
     expect_output_json: bool,
     md: &'static str,
+    files: &'static [(&'static str, &'static str)],
     expect_success: bool,
 }
 
+impl<const N: usize> mdq::OsFacade for &Case<N> {
+    fn read_stdin(&self) -> io::Result<String> {
+        Ok(self.md.to_string())
+    }
+
+    fn read_file(&self, path: &str) -> io::Result<String> {
+        for (name, content) in self.files {
+            if path == *name {
+                return Ok(content.to_string());
+            }
+        }
+        Err(io::Error::new(ErrorKind::NotFound, format!("File not found: {}", path)))
+    }
+}
+
 impl<const N: usize> Case<N> {
     fn check(&self) {
         let (actual_success, actual_out, actual_err) = self.run();
@@ -31,9 +50,31 @@ impl<const N: usize> Case<N> {
     fn run(&self) -> (bool, String, String) {
         let all_cli_args = ["cmd"].iter().chain(&self.cli_args);
         let cli = mdq::cli::Cli::try_parse_from(all_cli_args).unwrap();
-        match mdq::run_in_memory(&cli, self.md) {
+        let restore = EnvVarRestore::set_var("MDQ_PORTABLE_ERRORS", "1");
+        let result = match mdq::run_in_memory(&cli, self) {
             Ok((found, stdout)) => (found, stdout, String::new()),
             Err(err) => (false, String::new(), err.to_string()),
+        };
+        drop(restore);
+        result
+    }
+}
+
+struct EnvVarRestore(&'static str, Option<OsString>);
+
+impl EnvVarRestore {
+    fn set_var(key: &'static str, value: &str) -> Self {
+        let restore = Self(key, env::var_os(key));
+        env::set_var(key, value);
+        restore
+    }
+}
+
+impl Drop for EnvVarRestore {
+    fn drop(&mut self) {
+        match (&self.0, &self.1) {
+            (key, None) => env::remove_var(key),
+            (key, Some(old)) => env::set_var(key, old),
         }
     }
 }
diff --git a/tests/md_cases/file_args.toml b/tests/md_cases/file_args.toml
new file mode 100644
index 0000000..bbdc6ab
--- /dev/null
+++ b/tests/md_cases/file_args.toml
@@ -0,0 +1,52 @@
+[given]
+md = '''
+- from stdin
+'''
+files."one.txt" = '''
+- from one.txt
+'''
+files."two.txt" = '''
+- from two.txt
+'''
+
+[chained]
+needed = false
+
+[expect."read one file"]
+cli_args = ['-', '-oplain', 'one.txt']
+output = '''
+from one.txt
+'''
+
+[expect."read two files"]
+cli_args = ['-', '-oplain', 'one.txt', 'two.txt']
+output = '''
+from one.txt
+from two.txt
+'''
+
+[expect."read a file twice"]
+cli_args = ['-', '-oplain', 'one.txt', 'one.txt']
+output = '''
+from one.txt
+from one.txt
+'''
+
+[expect."explicitly read stdin"]
+cli_args = ['-', '-oplain', '-']
+output = '''
+from stdin
+'''
+
+[expect."explicitly read stdin twice"] # will only read it once!
+cli_args = ['-', '-oplain', '-', '-']
+output = '''
+from stdin
+'''
+
+[expect."file is missing"] # will only read it once!
+cli_args = ['-', '-oplain', 'missing-err.txt']
+expect_success = false
+output = ''
+output_err = '''entity not found while reading file "missing-err.txt"
+'''