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.

Sign up for GitHub

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

Improve download progress #355

Merged
merged 1 commit into from
Apr 25, 2016
Merged

Improve download progress #355

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions src/rustup-cli/download_tracker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,37 +121,31 @@ impl DownloadTracker {
}
/// Display the tracked download information to the terminal.
fn display(&mut self) {
let total_h = HumanReadable(self.total_downloaded as u64);
let total_h = HumanReadable(self.total_downloaded as f64);
let sum = self.downloaded_last_few_secs
.iter()
.fold(0usize, |a, &v| a + v);
.fold(0., |a, &v| a + v as f64);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of making this a float? this is simple addition of integers, there's no rounding here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is just a matter of taste. I just wanted sum to be a float and I tend to apply int-to-float casting as early as possible when I know the end result is a float.

let len = self.downloaded_last_few_secs.len();
let speed = if len > 0 {
(sum / len) as u64
sum / len as f64
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for consistency with the below i think this should be sum as f64 / len

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f64 / usize doesn't compile

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then i guess you want sum as f64 / len as f64, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a sense, yes. But in my version, sum is already of type f64.

} else {
0
0.
};
let speed_h = HumanReadable(speed);

match self.content_len {
Some(content_len) => {
use std::borrow::Cow;

let percent = (self.total_downloaded as f64 / content_len as f64) * 100.;
let content_len_h = HumanReadable(content_len);
let content_len_h = HumanReadable(content_len as f64);
let remaining = content_len - self.total_downloaded as u64;
let eta = if speed > 0 {
Cow::Owned(format!("{}s", remaining / speed))
} else {
Cow::Borrowed("Unknown")
};
let eta_h = HumanReadable(remaining as f64 / speed);
let _ = write!(self.term.as_mut().unwrap(),
"{} / {} ({:.2}%) {}/s ETA: {}",
"{} / {} ({:3.0} %) {}/s ETA: {:#}",
total_h,
content_len_h,
percent,
speed_h,
eta);
eta_h);
}
None => {
let _ = write!(self.term.as_mut().unwrap(),
Expand All @@ -168,20 +162,36 @@ impl DownloadTracker {
}

/// Human readable representation of data size in bytes
struct HumanReadable(u64);
struct HumanReadable(f64);

impl fmt::Display for HumanReadable {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
const KIB: f64 = 1024.0;
const MIB: f64 = 1048576.0;
let size = self.0 as f64;
if f.alternate() { // repurposing the alternate mode for ETA
let sec = self.0;

if sec <= 0. {
write!(f, "Unknown")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the condition to print "Unknown" is now wrong...

} else if sec > 1e3 {
let sec = self.0 as u64;
let min = sec / 60;
let sec = sec % 60;

if size >= MIB {
write!(f, "{:.2} MiB", size / MIB)
} else if size >= KIB {
write!(f, "{:.2} KiB", size / KIB)
write!(f, "{:3} min {:2} s", min, sec) // XYZ min PQ s
} else {
write!(f, "{:3.0} s", self.0) // XYZ s
}
} else {
write!(f, "{} B", size)
const KIB: f64 = 1024.0;
const MIB: f64 = KIB * KIB;
let size = self.0;

if size >= MIB {
write!(f, "{:5.1} MiB", size / MIB) // XYZ.P MiB
} else if size >= KIB {
write!(f, "{:5.1} KiB", size / KIB)
} else {
write!(f, "{:3.0} B", size)
}
}
}
}