WIP: Fall back to plain output when colors are unavailable - #3946
slash-aech wants to merge 2 commits into
Conversation
Signed-off-by: slash-aech <harshitbenke@proton.me>
slash-aech
changed the title
| writeln!(t, "{}", title).unwrap(); | ||
| writeln!(t, "--------------------------").unwrap(); | ||
| t.reset().unwrap(); | ||
| let _ = t.reset(); |
There was a problem hiding this comment.
this unwrap related to terminal at all as term::stdout().unwrap(); above, we need to handle these unwraps and do not ignore them.
There was a problem hiding this comment.
Hey, I went through src/bin/cmd/server.rs to use the reference as how the errors are handled and used following type for the same
if let Err(e) = t.reset() {
error!("Failed to reset terminal: {}", e);
}
Lemme know if that works or I'll change the approach to maybe explicit error handling using the Error enum at the bottom of the code as next step
There was a problem hiding this comment.
Or if the errors should be displayed in the terminal that would be worked upon
There was a problem hiding this comment.
this is not what I asked, we should handle all such errors (unwraps) at such function, unrelated to this PR btw.
There was a problem hiding this comment.
Oh, if you'd like it to be handled it with return type Result<(), Error> maybe I can try to propagate the issues with the ? instead of just logging, although I'm not sure if that change would be a breaking change and might lead to bigger change than apparent.
Mind letting me know what steps I can work on now and types of changes can help the current case?
There was a problem hiding this comment.
yes, it means we can switch to println if terminal has problems
| .unwrap(), | ||
| }; | ||
| e.reset().unwrap(); | ||
| let _ = e.reset(); |
There was a problem hiding this comment.
Same as comment above, this unwrap should not be ignored
|
Thank you for the review, I'll get to them ASAP and let you know. |
Signed-off-by: slash-aech <harshitbenke@proton.me>
2 participants
Fixes #3925
Please note the changes in file
src/bin/cmd/client.rs. Changedunwrap()tolet _ =to ignore the error and fallback to plaintexts.Uses this pr and reference for the changes.
Is not a breaking change
Does not contain new tests