Panic-less code and user prompt for toolchain installation#30
Panic-less code and user prompt for toolchain installation#30schell merged 2 commits intoRust-GPU:mainfrom
Conversation
tombh
force-pushed
the
panic-less-code
branch
from
8adf52a to
02311f1
Compare
tombh
changed the title
tombh
force-pushed
the
panic-less-code
branch
from
02311f1 to
ee1993a
Compare
schell
left a comment
schell
left a comment
There was a problem hiding this comment.
Just a quick fix about main logging the error chain and producing a backtrace for better debugging.
| reason = "CRAB GOOD. CRAB IMPORTANT." | ||
| )] | ||
| { | ||
| print!("🦀 "); |
There was a problem hiding this comment.
We can totally change this BTW. I just used Ferris to as an example of what we could do.
crates/cargo-gpu/src/main.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn main() -> anyhow::Result<()> { |
There was a problem hiding this comment.
In general I think it's fine for a cli program to panic when it hits an unrecoverable error. There are benefits to this behavior like knowing exactly where in the program the failure occurred since the line number is included in the print statement and a backtrace is produced.
But wrapping all possible errors in Result can be nice for testing and is a generally good practice.
So - my only concern here is that if main returns the Result, we have no way of logging the error (and the errors further up the chain of errors) or the backtrace and instead we rely on whatever Rust's default behavior is - which I think is likely just panicking with the text of the top-level error, erasing the backtrace.
So I think here we should probably not return Result and instead if let Err(msg) = result { ... } making sure to use all of anyhow's machinery to print the relevant information.
There was a problem hiding this comment.
Good points, I agree. I've refactored so that main() wraps and captures the error from a run() function, then writes the error message to stderr and logs the backtrace when not a release build.
Also add some user output for the major steps of installing toolchains, compiling `spirv-builder-cli` and compiling the actual shader. Surprisingly there's no way in the standard library to get a single keypress from the user! There's ways to get a line, therefore a keypress then a newline, but that's not so conventional for responding to a "y/n" prompt. Fixes Rust-GPU#14
tombh
force-pushed
the
panic-less-code
branch
from
ee1993a to
9dc0c03
Compare
2 participants
There's now prompts for when Rust toolchain assets need to be installed. Surprisingly responding to a prompt with a single keypress requires a whole crate, namely
crossterm.Also all the code is now "panic-less", so no
unwrap(),panic(), etc. Everything goes through?thanks to theanyhowcrate.Fixes #14