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

Fix: Code is actually reachable #286

Merged
merged 1 commit into from
May 20, 2021

Conversation

matthiasbeyer
Copy link
Contributor

This fixes an reachable unreachable!() statement.
This might not be the approprite fix, although it resulted in no panic
in the code I used to test it, so it seems appropriate as a first idea
for a proper fix.


If you think this is a good first idea, please release it as a patchlevel update (v0.16.1)!

Related to #285 (but there might be a better fix)

This fixes an reachable `unreachable!()` statement.
This might not be the approprite fix, although it resulted in no panic
in the code I used to test it, so it seems appropriate as a first idea
for a proper fix.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
@mibac138
Copy link
Contributor

mibac138 commented May 17, 2021

Turns out is_hidden must return false for ProgressDrawTarget::Remote because otherwise progress bars won't notify the MultiProgress about their state changes thus deadlocking the program. However, a progress bars notifies a MultiProgress by (among others) sending it's drawn state and to draw this state width is required (for wide_bar and wide_message). MultiProgress just forwards fn width to draw_state.width which actually might be Hidden and actually reaching the 'unreachable' branch. I believe this is the simplest solution to this issue as otherwise we'd need to add a new mechanism that skips drawing when a progress bar's draw target is remote and that remote's target is hidden, which seems relatively complex compared to this one line fix.

MCVE:

use indicatif::{MultiProgress, ProgressDrawTarget, ProgressBar};

fn main() {
    let mpb = MultiProgress::with_draw_target(ProgressDrawTarget::hidden());
    let pb = mpb.add(ProgressBar::new(123));
    pb.finish();
    mpb.join().unwrap();
}

@djc djc merged commit 1047de9 into console-rs:main May 20, 2021
@djc
Copy link
Member

djc commented May 20, 2021

I just published 0.16.1 which contains this fix (along with a few other ones).

@matthiasbeyer matthiasbeyer deleted the fix-reachable-unreachable branch May 20, 2021 09:53
aj-bagwell pushed a commit to aj-bagwell/indicatif that referenced this pull request May 20, 2021
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

Successfully merging this pull request may close these issues.

3 participants