-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add progress bar #2309
Add progress bar #2309
Conversation
@ry a little suggestion from me: can progress bar be disabled after initial boot? Once we support |
@bartlomieju I'm not sure about that... I would expect that if I |
@ry after second thoughts you're probably right. Re disabling of progress bar, maybe it could be handled by pub fn unset_callback(&self) {
let mut s = self.0.lock().unwrap();
assert!(s.callback.is_some());
s.callback = None;
} |
@ry regarding a TODO from progress.set_callback(|completed, total, msg| {
eprint!("\x1B[K"); // Clear to end of line.
eprint!("[{}/{}] {}\r", completed, total, msg);
if completed == total {
println!();
}
});
|
@bartlomieju I tried that at first, but learned that |
How about Taking into account two cases I presented before there would be 3 places to signal process bar that everything has been downloaded and compiled, and |
@bartlomieju As you suggested, I've added Progress::done() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good with a few remarks. I'll approve it like this, but please respond to comments.
@@ -682,7 +688,10 @@ fn fetch_remote_source_async( | |||
} | |||
}) | |||
}, | |||
) | |||
).then(move |r| { | |||
drop(download_job); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. What is this drop() thing needed for?
No description provided.