Repository: yshavit/mdq
Pull request #380 (2025-05-21 06:50:55)

don't use MDQ_PORTABLE_ERRORS env var
Instead, just use `#[cfg(test)]`.

`env::set_var` is not thread-safe.

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

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

## Walkthrough

This change eliminates the use of the `MDQ_PORTABLE_ERRORS` environment variable, both in the test harness and in the main code. Instead of toggling error message verbosity at runtime via an environment variable, the formatting of error messages in the `Error::FileReadError` variant now uses Rust’s compile-time conditional compilation: tests display the full error message, while production builds show only the error kind. The test utility struct `EnvVarRestore` and related logic are removed, as are references to the environment variable in scripts and tests. Import statements are also adjusted accordingly.

**Grade: A**

</details>

<!-- walkthrough_end -->

<!-- announcements_start -->

> [!NOTE]
> <details>
> <summary>⚡️ AI Code Reviews for VS Code, Cursor, Windsurf</summary>
> 
> CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
> Learn more [here](http://coderabbit.ai/ide).
> 
> </details>

---

> [!NOTE]
> <details>
> <summary>⚡️ Faster reviews with caching</summary>
> 
> CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure `Review - Disable Cache` at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the `Data Retention` setting under your Organization Settings.
> Enjoy the performance boost—your workflow just got faster.
> 
> </details>

<!-- announcements_end -->

---

<details>
<summary>📜 Recent review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**


<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between e40e83edb398ad922dce4a0df2f63029f2eeda40 and 47b9ba09c9c3af6a5ebfa2fa57aac91d929f428c.

</details>

<details>
<summary>📒 Files selected for processing (3)</summary>

* `scripts/system_test` (1 hunks)
* `src/run/run_main.rs` (2 hunks)
* `tests/integ_test.rs` (0 hunks)

</details>

<details>
<summary>💤 Files with no reviewable changes (1)</summary>

* tests/integ_test.rs

</details>

<details>
<summary>🔇 Additional comments (3)</summary><blockquote>

<details>
<summary>scripts/system_test (1)</summary>

`89-89`: **Environment variable removed correctly.**

The removal of `MDQ_PORTABLE_ERRORS=1` is consistent with the PR's goal to replace runtime environment variables with compile-time conditional compilation.

Grade: A

</details>
<details>
<summary>src/run/run_main.rs (2)</summary>

`10-10`: **Import dependency made explicit.**

Explicitly importing `std::io` is good practice as it clearly shows the module dependencies. This also supports the removal of the env import since it's no longer needed.

Grade: A

---

`97-107`: **Good use of conditional compilation.**

The implementation smartly replaces runtime environment variable checking with compile-time conditional compilation. The behavior is preserved - tests get the full error message while production code only sees the error kind - but without the thread-safety concerns of environment variables.

The explicit `Ok(())` return improves code clarity.

Grade: A

</details>

</blockquote></details>

</details>
<!-- internal state start -->


<!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpETZWaCrKNwSPbABsvkCiQBHbGlcSHFcLzpIACJafAwAclDsRA8AWQARAEUAfSsAeRtoAEEAIQAZAFEcypsbQoBlSDIJSAlnaMgAdzRkBwFmdRp6OTDYDxTKSFlEWDQJdXRkW0gMRwEpgGYADgAGDRhxyGZtLAY5jFIeCnxufFT6VAJ7Am5IFPhLsY8W+BuMNgYULtCjwFSRSAAA0yuQKRTKVRqdUakPQGEeGEQNDQ9H8Xnk8W+kBsKVC1FwoIEeA8kLMAG0GAAzIgAChoWIAlABdVGM/B8Jjo9TweJoXxMZjceBeagirC0bCgr7s8SXA7uY74cTtcSEvl8XAIZDnTBXJ5zUKGjyM7AYMRyqEtDgcVK4HIg1GTejPV3NDALf6A4HOMECSLIVAYLVjfw4sCINCMkgAGm6CHOjEwkEiOLC+BQiAc0hQZ3iDEV/iBYRCzQAHqI8HL1UcVtpmMhnvBJTcpESE0ncPJMLiSF5Q9L1ATGUSVfYSLhsG9Rv5mPgFsqjkpuGQlHaCVhfoH2G0Q+Di8P3ohPkRiaSEjJsNLcGBPtWsQoMIz4ERFbLCeTKWpZsPG4bxfH8IIawYLMNkgPlbXoahIFgXBcG4RAOAAekwoh1FgbABA0CVMJmOYFlwTDmFoAJMNAnxMJ2fYjH0YxwCgHd+GnNA8EIUhyCoYYFFYdguF4fhhFEbVi1GJglCoVR1C0HQWJMKA4FQVAs24ghiDIZRBIlIMuCoLp7EcE4XEgGTFGUBTNG0XQwEMVjTAMRAGFBbhcEQTDEBmGhmByFUOAMaIwoMCxIGKABJXT+OoKIHCcSz8GnE1LmkNwjkPeIgxPUEzyhGE8kKEoKmqWp6hsBoAF4AEZUR6ZAVzXKJGRuZgiUMi9PgkfBoN1LBDSQihbQ7I5ISogJURIety1wQqujwvw6D+STEoW9FnAxUCyXRTMvMVYt1GQfA8F2tF6EoG4+CxWN22AoSTn22aG0Gy7mgoG7kOHMcvi8fBcIYFaTlfW10tIWhUyWw0upYZ7aHvMyGAYaRTr4RltC8I6XmlXwlBoChBgwa8iUhNAxGwMUcgcFG0d5GUiAOAA5fMtXGPhV1oeAvwGuUO3zK0PwpfBfEZAGulTa7+R+9E/qIVMZbOtC8Gufq0dJrpKA8E4lA0IxzEsYovEJv9MTzIklAYGUBP5zi6zuChBJl0Cw3gYH2GFTKDFZ8hQvCgwIDAIxEAoBhMNGjAI9tHJQYwDQKAw/3ogio3Yr4/TEvM5wp0YC5SEQLKPAhnXFB5+BiyFyEMiQbgZVkVEuzrkggzNuCZaryovv5Z0ADFpRIGwSBxLubtREEwSBA4rH8BYzsQfFUyF/UTlQ0nUqJaXObRtAri3HcokJIWcoBY8J8K6FshK+FyqRKqGkhLgeeaSVB1TRZeE+QTl7Az7vrYQsu8SAAG5+BWgoEtVI79QifyBIfDA+JN7dz4AAa0+LQR65BTKyQ8P4OuFNK5Gj8LacQbA/QBlysec4ogUHdGWhKKUkQwCkOLvEbmg0xRCUYW3AC8AqTsi4AqJUN4VTIBZLSBkzI2QhG5JCDk0Drhf3GtaX+W9jg7yuHdUm+oeD8gWmoMcg4PrWxDEY18X8SBEFtkfEIiBQHKyvEoe2ojIDiPpEyVkUZcDSM5LI+RKAYFKm8vwBB8hj7IMgGg9EmCSCmSbpEVu70xSIHzLNOu7t1CIP8AuCg5tIT5BQSyFkHIOSokTITbooJVQiKOAAhMpApaYkVKTQUV4sTHkhIyZgnAOBDwcCbVEFxaDywOMUWg7C5RikXkSJueiXgJSDMgLW/h0C0CEKSKIzwWq9jQCtBUW0qyQhaI3SUcyLw4kQgeWs6SGCLEhFiWgzoRQnMdpoA2qcoom30nbZ4QsrY2zNqdacaS9GHz4K7McHsgRe0LoHSArM86mkroLI4V4iAYGoEdIFDtQX0BdgRSFfpwgVyWdrY4OISD6zCinQOrlRGYQsUQIKIQE5J2pR8mKcVM70CShZXOJdYUam5oyac2zCE/GhSsyElR/QADVnB9IIP4VEd1sBiA+idJYKTbkJUeJKBJ7AzbQ3TLAbovQVqIDuJiPhEIdEBUdiGRBrpqmb39H8ShVZz5hhpMVOEZVESVRRBbSE0Q6rRFREI0mx85qNkJBvKuABhXoJBnSR1RGwQ0igPr+CxPyUmmq824Qxb4do2MPDlMoD0CgGDIAJv5Dmq13NLjTKFtBHwwaZUSHlRQRV/IU0unnO6ZwLJoi+tKgiCqyJqrRFTKG6IpSPrRpuXc2gNxuAshzUqkgC6tkkCkE7V1FDT6etPN69AKylCREEu1FgZM03qMzTWjUm7nFHFnADIG787TYybTeBN5Rornp/HlbgzgryXCXlBZNdY3pylTBeZWu1MJqOgodYRqYVynGQODfOdB9YG0isbU2g0BaW1EACkj9sQVOzBZ4N2ULiXeygEPVcpbX0eFVeq6VcqFUhD7WU/amrejarBIJDNsBFDIAvNXNdLyDVAjbjezqkJ6WMuZViVlkJ9ZQARQKi2Qs0UYpycWDemB5DUeduCgl7s4Lg0ozLcTkmqUBxYgYVSfo8VcR4lygSURDIiT8GgUyvKc5WXkDg+Sah7LKScm5tiT11A5HgLQRAORZ4Vy1rQGmC0D2ufcwAFgAOwCAAJwCDQLsErDAqubETAANjQAAVhIAITGAAmTGjXCtoApiVuqtASttZK4yfLbXtjAzy/FwyiXkupfS7EugOQOL6CAA== -->

<!-- internal state end -->
<!-- finishing_touch_checkbox_start -->

<details open="true">
<summary>✨ Finishing Touches</summary>

- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> 📝 Generate Docstrings
<details open="true">
<summary>🧪 Generate Unit Tests</summary>

- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Create PR with Unit Tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Commit Unit Tests in branch `rm-env-var`
- [ ] <!-- {"checkboxId": "07f1e7d6-8a8e-4e23-9900-8731c2c87f58", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Post Copyable Unit Tests in Comment

</details>

</details>

<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

<details>
<summary>❤️ Share</summary>

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

</details>

<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=380):

- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  - `I pushed a fix in commit <commit_id>, please review it.`
  - `Explain this complex logic.`
  - `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 explain this code block.`
  -	`@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 explain its main purpose.`
  - `@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.`

### Support

Need help? Create a ticket on our [support page](https://www.coderabbit.ai/contact-us/support) for assistance with any issues or questions.

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 generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR.
- `@coderabbitai generate sequence diagram` to generate a sequence diagram of the changes in this PR.
- `@coderabbitai auto-generate unit tests` to generate unit tests for this PR.
- `@coderabbitai resolve` resolve all the CodeRabbit review comments.
- `@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/run/run_main.rs b/src/run/run_main.rs
index ebb1a4e..925404d 100644
--- a/src/run/run_main.rs
+++ b/src/run/run_main.rs
@@ -7,8 +7,8 @@ use crate::select::Selector;
 use crate::{md_elem, output, query};
 use pest::Span;
 use std::fmt::{Display, Formatter};
+use std::io;
 use std::io::Write;
-use std::{env, io};
 
 /// The run's overall possible error.
 #[derive(Debug)]
@@ -94,11 +94,16 @@ impl Display for Error {
                 writeln!(f, "{err}")
             }
             Error::FileReadError(file, err) => {
-                if env::var("MDQ_PORTABLE_ERRORS").unwrap_or_default().is_empty() {
-                    writeln!(f, "{err} while reading {file}")
-                } else {
-                    writeln!(f, "{} while reading {file}", err.kind())
+                #[cfg(test)]
+                {
+                    // For tests, use a simpler, more portable error string. Especially useful for integ tests.
+                    writeln!(f, "{err} while reading {file}")?
                 }
+                #[cfg(not(test))]
+                {
+                    writeln!(f, "{} while reading {file}", err.kind())?
+                }
+                Ok(())
             }
         }
     }


Test patch:
diff --git a/scripts/system_test b/scripts/system_test
index 3dd2b65..7a03a4c 100755
--- a/scripts/system_test
+++ b/scripts/system_test
@@ -86,7 +86,7 @@ function run_test_spec() {
 
     local actual_success=true
     set -x
-    MDQ_PORTABLE_ERRORS=1 "$mdq" <<<"$stdin" "${cli_args[@]}" >actual_out.txt 2>actual_err.txt || actual_success=false
+    "$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 ccb6a58..94ab53d 100644
--- a/tests/integ_test.rs
+++ b/tests/integ_test.rs
@@ -1,6 +1,5 @@
 use clap::Parser;
 use mdq::run::Error;
-use std::ffi::OsString;
 use std::io::ErrorKind;
 use std::{env, io};
 
@@ -66,7 +65,6 @@ 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::run::CliOptions::try_parse_from(all_cli_args).unwrap();
-        let restore = EnvVarRestore::set_var("MDQ_PORTABLE_ERRORS", "1");
         let mut runner = CaseRunner {
             case: self,
             stdout: vec![],
@@ -76,28 +74,8 @@ impl<const N: usize> Case<N> {
 
         let out_str =
             String::from_utf8(runner.stdout).unwrap_or_else(|err| String::from_utf8_lossy(err.as_bytes()).into_owned());
-        drop(restore);
         (result, out_str, runner.stderr)
     }
 }
 
-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),
-        }
-    }
-}
-
 include!(concat!(env!("OUT_DIR"), "/tests/integ_test_cases.rs"));