Repository: yshavit/mdq
Pull request #301 (2025-04-01 03:55:33)

unify the two IO implementations
Add stdout and Error handling to OsFacade. This lets us unify the two previous implementations (`run_stdio` and `run_in_memory`).

In service of #213.

coderabbitai[bot]: <!-- This is an auto-generated comment: summarize by coderabbit.ai -->
<!-- walkthrough_start -->

<details>
<summary>📝 Walkthrough</summary>

## Walkthrough
This pull request refactors how output and error handling are managed by updating the `OsFacade` trait and its implementations. Two new methods, `get_stdout` and `write_error`, are added to `OsFacade`, replacing previous direct output calls. The legacy `run_in_memory` function is removed, with its responsibilities partially integrated into the new `run_generic` function, which now uses the helper function `run_or_error` to consolidate input reading and markdown processing. In addition, the implementation for the `RealOs` struct in `main.rs` and the integration tests via the new `CaseRunner` struct have been updated accordingly to accommodate these changes, ensuring a consistent approach to managing outputs and errors across the codebase.

## Possibly related PRs
- **yshavit/mdq#289:** Addresses similar modifications to the `OsFacade` trait by adding the `get_stdout` and `write_error` methods, with a corresponding update in the `RealOs` struct for handling output and error reporting.

Grading: A

</details>

<!-- walkthrough_end -->
<!-- internal state start -->


<!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpADK2VmgqyjcEj2wAbb5AokAI7Y0riQ4rjedJAARNgY8ABm8riwnrgA7viQAJIA8ijM3FFsGLjU8PgYiDGQGWjIiNgCzOo09HLhaZDYiJSQsoiwaBLq6Bj0qPBKZUnw0Q2QGE4C/QDMAAwAjBow3czaWIIiYpKe+IldqLYoyASQZMMYDOndeYgAYmgMaEqQnb9aPAMERIE9aN5gaDEvg+IhyhMXPR8HhuHhIAAKeG0FG4ACU43olAosMg8ICaGYiF2cFQzEUcx+4iq6HgVPC2XiSRS3Uy2RIAA8kOIQYViiRSuVmdUADRLSkkbzyAAGFHiAH1sZVlYTIKqNcD1Wx6a5lTTugwnqRkLx8Nx8H1JlhUtc7C5PMCaBNovdAQFEMgkE1PGYAExbNa7D6kxLYCipfpKcrwbyIOUJ6x2H5YVboBgvAMLMIZmG+fAZKGQACqNgAMlwANqwXC4biIDgAeg7RHUsGaGiYzA7g2Go1wHeYtECHbRvg7my2AF0Mc3W+2uz3Uv3B8OhiN1BOpzOfN559s8RojPpjOAoGRkZc0HhCKRyFR2gpWOwuLx+MJROIUgyPITBKFQqjqFoOjXiYUC0oGyCYDgBDEGQygfoOkpcFQGRkk4ByuP8IGKMoEGaNouhgIYN6mAYiAUAwHaQgIGgUO2BgxJxBgWJAACCOSoW+1DRE0ziERcjBWtI7gWlJgZlCStDYC84RZEsJC4WwqSKHc2QZsq7xfD8Sg6rgVDqFwyqkLgmq4DieAYgAZMw6J9N4iQElRYp+AA6hQ6gkDqmD0MqGT+TQ6rErCTkuWEbmJHKxJcAAohQJIUHiZp7J4+oYOqhrGrCsg6rGzzSmCiyrGQ/gSvgUhErMARKnKwUoLgyClacVRoJCuDyMMMgkNV3AuOIPVKigZQkEQ77RJ6endOQuG5eqr6UPADAlfEXUYHKGQIJaSzluEaAANbSOgkCxSoUQ1YklBkCp9zKg6WUeA8aWkuCkKigN/xDVg9JAok8y0AA3JN8IkL8/CXLwnqVlFbGQECARiM1XQ5WFAWRZ9FA6lpsCKLcjDjT62TfZ4SPUtlepqnla3+ZtkCdeVGDHT8viIepuFpN43D9KzlS7XTGqwrj6XKntB2wAo1T4JCtDCXc3QUkCooScCaJhK1Ga2gWiCVhJGYEadOIZFgTBTWU5o5fT6ri0jW1lcLNW/IhviTdrPSG6KL3Umr6rjU5DCQplur69IfSBrgcoBLgcYJKKaD/PgCvQ1g/o+JotO4t732Vn9VVYNg3BKx+9y9DlDoaNZtn2bgGLh8CUMwxJKe8CQowoogE1KCD5DIqi6JC1Uux5FIVC+OmsmYNaZJmdDzA/VTeMVRMP2gq1efogcGBoKQkp1Agt0BPSoyivp9sFbVpos9t0qXkY3GWLx3g0O+wu6ZjKOiN4LgVCqMgCSgp7TxmiKSNEAhIQMAeLMcQ0kDBQAALIkG0vQQEdBLKJCwHXbEuIYquUVB5XQehvKQD8gFHUwJwjmTCAZT43xfiBUmnqeijFmKsUQMqJBkBUHoPQLQJQtBsFYGxhFJGhC4rEMSmlFKeNm7OjoXqQyTCTKsOVOwpi8AWJsR4VAD4D9XaGyIPvBOAQehl2EiIvUUCWaZw1FqfAGJQ7wC4I5AAwpCOUDouBsmKJAVRxkSCeTIQINO3hqFYE0QxbRujuGQEAEmEtjmj2P8BqRmG0XGQncV4+APj2yQGcuifxfggnMNCanBWUS2GxM4Xo3hhiXYshMWYuMnhS7lywSkgQaTr55UKq4bJbiil5IKX4ooZTGHBMqTYaQ2dgAYnCQrOU9gzJQjxHKVK6U9A1JiRwnRXCdTJLPnVOa0StH1O4Y0oxLT4CmOoO0yxXSbHKhwekjAwAfJyg+HoYZuTvEL3VuqK2XpOAOHWSCOUddcRcA+LM+Z79gDLO8FsvGuyNGXMOXopJNVz7nNqQc+J+jIBNJ2oI4RoiPkOwoBLaKriAX5P4IU4pYRSmBOmRU0hkA5lNCRSitFOy9lYuJRxLiSDaJaIOMCLhHAxUxBfnxASjNrF4TEvICSlo56IPelqkEF0aEZlAh6DAEgFZSCugyEGTIv4ch/v42E7VdQZlKRKdggCjiXH0uU9RZltBhBhHwfScyervB1OSZSOd3oOvjAvYSR8i4A2eaq+4wJQ7YD+MqbZsISokmYD/ZUk5AgE0UD4E1rcHyizyk4rKvEhHqGFuNWQM8crYg4BwbUYpHXIFHJ4YuDwBQjW9PQVNzxvAZtbXZXEWUjA+V7Ia7oDCjLMOoZMt1ZQPXpjUktK6aCia0B7SMPtSbMGvLwVOvAUtj4bVlvHRO3NvD4AYOdeglM7UZnhMFJE/Bh6x11KFcKJA6X42ljek651ELROzfjcIsgBYR38mUGO77uifsRBQIka8d5Rv2Ice+zSMAAHJkCPp7LAxN1Ueof3JjVYo3xXieE5n4Z69sa11F7JdJjdrC1TnbfbTJm05QjQDJWFO11oGeACPdAIzx0jZGVCG7wYbzSoD1aQS1wN5g9uOkalge9aBgBXpND+iR6Mk0FKIPAdAhP4DBfAJt6BEj3VOJfboZyp5+FBSSPwiRH24WNt0NA3BigbQ9U/Ceyhp4/zUxdR4mAnrdFHvvXqGqvWLp9SwzotpRjqy3vW6UPVd3oI6qSbD68ISiYmB9dKtHHVQlA7dHqiBshUfWq5qmApLPlUJsTBdnhpVYCS0/cwr937oVtfcDMShQ4AOlMAy4oDHUQL4FAmBcCIhaavHwvdxNT1UvPQ3KRZJiGVPZZUdtlCaA1IjWIPUimw2YtiQNo5vD+H7opd0t5YjAPAaO/FWRFB5HpUUQvNUt2FPQyUwkmh+yJyHBewY25WBWmPIsZ06xlkkjUqcSHQF93EAg8LYcZ25KYdSvhzi5JyosdFr4xktCTNcdMtZTyyH7xCcDZJ+VMnT2KfXM4gqiVYAjA0HhIgDsnpprqlF5oNicqBeKv4oJdCIl8IuFS5JbViAZKMbksZxSylPApx3Td+hHiGgkBsPEN8plhhhEekFvlysf5JZ6uoeQndu69Amg0ZrDB7MfgrKkAt5u+jhrMpG1humpoCjCJrNdkpKz6VpxwDLpk6G2z1KHy31vKDh7B2EIm3gD2XSkw9WTdqU7Kmz8AAAcrslqj7RRB9ln0Eas0WbzGLyVvgmiL24CClVvvYF8aZ9dZKD1sMC0p7T7Q/1JN0cV2yHaFX/BonZ6txgG3kMaBt0fHdcvT15PZ5nQYd6qpoa0HrsCAmO36Ds1wt8A2F1k+0BKl3kdsASTYCILLfSPww8D8ZMVIJJq8LdN8bdM8L9fh1QQYohb8BFDZWh/4KAfd8wo4X9F04DpB39FR6BEg80C0ACWEy9gCSAaRt0NIit91EILFT0UMcoZ9OV1Fx93VpRLIDtp1QNDo70KBqhLpxNT4SBpNHo5MC18FL1/hsAnNKAWoh9xEgMnZuDZYgsBYJhkAkZd0AwD4X8FpJ0R988oQn4chnRF1LRRBToED3sAscoT9QdI0W0yR7k2kLE6QrVQZ30zojcro8AbpJNhDD8xDNFiE3puhSMNoWZSR6Yk5QRdNnAqsKNM5hDvgCAAgR1sgGAKQaBxgd94tzhLgwC+gIC89lCSYEwsBhNHQGC9QU9+MGcNoudhZM8s535ytN4F8rEl9Jp00/h9IJCB9/1sRiRc0WAQ9wDc8QMapzEYjMY+hIAJAeoQgPYqhQQW8f4Wi480tGM9NWoLMGA8AmijAIsPNHCYsNCMAngEtPBTcnlWo29ZtXZNUqgXg+DkAW8+sFAlBG8/NKxA1/oWx+g94dCj4JI0MlYMMf1Wx0Rdi14C4QRhtFcxtP4gFqjpsUCPV5sB0wEPxIFmg1t2AG1EEoBa8qDTcPtXkySN8JjgBCM0A5QrZ4RIBa8uBeh4AAAvEgDFGHGXcXSXIgaXUIBHbbRA5w1HDpToz7UpYABksIZkn2dkzkq6XjVPZgzwP4zxC3OvLk6JHkiXKafkmXI5XFanSZaUoBWUlkw2DkshJg5dP4P4qkrfSgGk9UOUevPZXUvkgU+EF7BXCVaiWCOBStJ8FCFVDCPTb8fwNAXCUSAieQToY1cCNQciaCKiAwQMzCdQfKA9dUAIUYDSOgWyUaSiAM28B4NYAADkrIYAADY1gGABAAB2DYDYRIWgRIRsgAVgABZ6zVA0ANhOyNgBANgKzGzGyABObshgIcgQbsxsksmiKATMmyaYRAXMrueYDIQs+8EsoAA= -->

<!-- internal state end -->
<!-- tips_start -->

---



<details>
<summary>🪧 Tips</summary>

### Chat

There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=yshavit/mdq&utm_content=301):

- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  - `I pushed a fix in commit <commit_id>, please review it.`
  - `Generate unit testing code for this file.`
  - `Open a follow-up GitHub issue for this discussion.`
- Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples:
  - `@coderabbitai generate unit testing code for this file.`
  -	`@coderabbitai modularize this function.`
- PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.`
  - `@coderabbitai read src/utils.ts and generate unit testing code.`
  - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.`
  - `@coderabbitai help me debug CodeRabbit configuration file.`

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

### CodeRabbit Commands (Invoked using PR comments)

- `@coderabbitai pause` to pause the reviews on a PR.
- `@coderabbitai resume` to resume the paused reviews.
- `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
- `@coderabbitai full review` to do a full review from scratch and review all the files again.
- `@coderabbitai summary` to regenerate the summary of the PR.
- `@coderabbitai resolve` resolve all the CodeRabbit review comments.
- `@coderabbitai plan` to trigger planning for file edits and PR creation.
- `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository.
- `@coderabbitai help` to get help.

### Other keywords and placeholders

- Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed.
- Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description.
- Add `@coderabbitai` anywhere in the PR title to generate the title automatically.

### CodeRabbit Configuration File (`.coderabbit.yaml`)

- You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository.
- Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json`

### Documentation and Community

- Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit.
- Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback.
- Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

</details>

<!-- tips_end -->

Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 2707260..a081d3d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -102,6 +102,8 @@ impl Display for Error {
 pub trait OsFacade {
     fn read_stdin(&self) -> io::Result<String>;
     fn read_file(&self, path: &str) -> io::Result<String>;
+    fn get_stdout(&mut self) -> impl Write;
+    fn write_error(&mut self, err: Error);
 
     fn read_all(&self, cli: &Cli) -> Result<String, Error> {
         if cli.markdown_file_paths.is_empty() {
@@ -131,39 +133,22 @@ pub trait OsFacade {
     }
 }
 
-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_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, os: impl OsFacade) -> bool {
+pub fn run(cli: &Cli, os: &mut impl OsFacade) -> bool {
     if !cli.extra_validation() {
         return false;
     }
-    let contents = match os.read_all(&cli) {
-        Ok(s) => s,
+    match run_or_error(cli, os) {
+        Ok(ok) => ok,
         Err(err) => {
-            eprint!("{err}");
-            return false;
+            os.write_error(err);
+            false
         }
-    };
-
-    run(&cli, contents, || io::stdout().lock()).unwrap_or_else(|err| {
-        eprint!("{err}");
-        false
-    })
+    }
 }
 
-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(&stdin_content, &markdown::ParseOptions::gfm()).unwrap();
+fn run_or_error(cli: &Cli, os: &mut impl OsFacade) -> Result<bool, Error> {
+    let contents_str = os.read_all(&cli)?;
+    let ast = markdown::to_mdast(&contents_str, &markdown::ParseOptions::gfm()).unwrap();
     let read_options = ReadOptions {
         validate_no_conflicting_links: false,
         allow_unknown_markdown: cli.allow_unknown_markdown,
@@ -205,7 +190,7 @@ where
     let found_any = !pipeline_nodes.is_empty();
 
     if !cli.quiet {
-        let mut stdout = get_out();
+        let mut stdout = os.get_stdout();
         match cli.output {
             OutputFormat::Markdown | OutputFormat::Md => {
                 let output_opts = OutputOpts {
diff --git a/src/main.rs b/src/main.rs
index f7ba8d4..9ceaa88 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,8 +1,8 @@
 use clap::Parser;
 use mdq::cli::Cli;
-use mdq::{run_stdio, OsFacade};
+use mdq::{Error, OsFacade};
 use std::io;
-use std::io::{stdin, Read};
+use std::io::{stdin, stdout, Read};
 use std::process::ExitCode;
 
 struct RealOs;
@@ -17,12 +17,20 @@ impl OsFacade for RealOs {
     fn read_file(&self, path: &str) -> io::Result<String> {
         std::fs::read_to_string(path)
     }
+
+    fn get_stdout(&mut self) -> impl io::Write {
+        stdout().lock()
+    }
+
+    fn write_error(&mut self, err: Error) {
+        eprint!("{err}")
+    }
 }
 
 fn main() -> ExitCode {
     let cli = Cli::parse();
 
-    if run_stdio(&cli, RealOs) {
+    if mdq::run(&cli, &mut RealOs) {
         ExitCode::SUCCESS
     } else {
         ExitCode::FAILURE


Test patch:
diff --git a/tests/integ_test.rs b/tests/integ_test.rs
index 827ce90..0e710a9 100644
--- a/tests/integ_test.rs
+++ b/tests/integ_test.rs
@@ -1,4 +1,5 @@
 use clap::Parser;
+use mdq::Error;
 use std::ffi::OsString;
 use std::io::ErrorKind;
 use std::{env, io};
@@ -14,23 +15,38 @@ struct Case<const N: usize> {
     expect_success: bool,
 }
 
-impl<const N: usize> mdq::OsFacade for &Case<N> {
+#[derive(Debug)]
+struct CaseRunner<'a, const N: usize> {
+    case: &'a Case<N>,
+    stdout: Vec<u8>,
+    stderr: String,
+}
+
+impl<const N: usize> mdq::OsFacade for CaseRunner<'_, N> {
     fn read_stdin(&self) -> io::Result<String> {
-        Ok(self.md.to_string())
+        Ok(self.case.md.to_string())
     }
 
     fn read_file(&self, path: &str) -> io::Result<String> {
-        for (name, content) in self.files {
+        for (name, content) in self.case.files {
             if path == *name {
                 return Ok(content.to_string());
             }
         }
         Err(io::Error::new(ErrorKind::NotFound, format!("File not found: {}", path)))
     }
+
+    fn get_stdout(&mut self) -> impl io::Write {
+        &mut self.stdout
+    }
+
+    fn write_error(&mut self, err: Error) {
+        self.stderr.push_str(&err.to_string())
+    }
 }
 
 impl<const N: usize> Case<N> {
-    fn check(&self) {
+    fn check(&mut self) {
         let (actual_success, actual_out, actual_err) = self.run();
         let (actual_out, expect_out) = if self.expect_output_json {
             let actual_obj = serde_json::from_str::<serde_json::Value>(&actual_out).unwrap();
@@ -51,12 +67,17 @@ impl<const N: usize> Case<N> {
         let all_cli_args = ["cmd"].iter().chain(&self.cli_args);
         let cli = mdq::cli::Cli::try_parse_from(all_cli_args).unwrap();
         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()),
+        let mut runner = CaseRunner {
+            case: self,
+            stdout: vec![],
+            stderr: "".to_string(),
         };
+        let result = mdq::run(&cli, &mut runner);
+
+        let out_str =
+            String::from_utf8(runner.stdout).unwrap_or_else(|err| String::from_utf8_lossy(err.as_bytes()).into_owned());
         drop(restore);
-        result
+        (result, out_str, runner.stderr)
     }
 }