Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail on non-zero exit status #18

Open
tomeichlersmith opened this issue Sep 1, 2024 · 4 comments
Open

Fail on non-zero exit status #18

tomeichlersmith opened this issue Sep 1, 2024 · 4 comments

Comments

@tomeichlersmith
Copy link

tomeichlersmith commented Sep 1, 2024

I'd like to add an extra check after the command has been run to ensure that it exited with a non-zero status.

(Context: it took me a long time to figure out I was just mis-spelling an option to the command I was running.)

The check would be put into

mdbook-cmdrun/src/cmdrun.rs

Lines 162 to 168 in d1fef67

let output = Command::new(LAUNCH_SHELL_COMMAND)
.args([LAUNCH_SHELL_FLAG, &command])
.current_dir(working_dir)
.output()
.with_context(|| "Fail to run shell")?;
let stdout = Self::format_whitespace(String::from_utf8_lossy(&output.stdout), inline);

and it could look something like

if output.status.success() {
  return Err(NonZeroStatus(&command, &output));
}

And NonZeroStatus would just be an error type a la https://doc.rust-lang.org/rust-by-example/error/multiple_error_types/define_error_type.html
(Sidenote, this seems like a common thing maybe this is already implemented somewhere and I missed it?)

@tomeichlersmith
Copy link
Author

I did not miss it, there is a nightly feature that converts does this non-zero status error by introducing a ExitStatusError type.

https://doc.rust-lang.org/std/process/struct.ExitStatus.html#method.exit_ok

I don't think this program will want to use a nightly feature, but maybe?

@tomeichlersmith
Copy link
Author

I wrote this issue before recognizing that many commands that exit with a non-zero code still have outputs that are nice to have within documentation pages. Maybe this becomes a configuration option or a different "flag" a user could provide to cmdrun.

e.g.

<!-- cmdrun --strict command -->

Would require the exit code of command to be zero.

@tomeichlersmith
Copy link
Author

Another option I like would be similar to the bats run syntax.

Ignore the command's exit status (current behavior)

<!-- cmdrun command -->

Require the command to exit with status N.

<!-- cmdrun -N command -->

For example

<!-- cmdrun -0 command required to run successfully -->

@FauconFan
Copy link
Owner

Hi @tomeichlersmith !

I think this is a great feature that could be useful and used.
Unfortunately, when a preprocessor fails, nothing happens, the stdout of the preprocessor is just injected into the mdbook.
The behavior should be to "output" a nice error message within the mdbook if the output code is not the one expected.

For flags, something like:

-0 # expects 0 (implicit version, also special case of -N, displayed for example)
-N # expects return code of N (implicit version)
--expect-return-code N # expects return code of N (explicit version)
--strict # expects 0 (more explicit version of -0)

would be nice, having both short and long arguments for this is a nice and easy bonus

For the error message, feel free to invent one, there is nothing "standardized" in mdbook (to my knowledge)

Feel free to work on this, I'll review the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants