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

Progress and speed indication for downloads #71

Closed
crumblingstatue opened this issue Feb 23, 2016 · 9 comments
Closed

Progress and speed indication for downloads #71

crumblingstatue opened this issue Feb 23, 2016 · 9 comments

Comments

@crumblingstatue
Copy link
Contributor

It's useful to know approximately how much time is left until the download is finished, and at what speed we are downloading at.

The original shell script version of multirust provides this information.

@crumblingstatue
Copy link
Contributor Author

I have 2 unsubmitted implementations of this.

  1. https://github.com/crumblingstatue/multirust-rs/commit/90af77e06bdf5fd2590325d2fb3755ee4a497aa8
    This one naively implemented it using print! inside rust_install, then I realized rust_install is meant to be a library, so no user facing output should go there. I abandoned this branch.
  2. https://github.com/crumblingstatue/multirust-rs/commit/b32ad1083826fde95ccb4857d66524849174446d
    Trying to build an interface for library clients to subscribe to download events, but it doesn't fit well into the current architecture, so I ended up using Rc<RefCell<T>>. I'm not yet sure if it's even possible to implement this without interior mutability with the current architecture.

Advice on how to go about this would be appreciated.

@Diggsey
Copy link
Contributor

Diggsey commented Feb 24, 2016

Hi, I agree this would be a useful feature! It should be fairly straightforward to implement using the existing Notification system - I'd recommend adding a new enum variant called DownloadProgress or similar, which carries the progress information. The caller can then handle notifications of this kind as they wish.

Feel free to ask here or on IRC if you have any questions!

@crumblingstatue
Copy link
Contributor Author

One problem with using the notification system is that Notification implements Display, but download progress isn't necessarily something you want to "Display". For example, a GUI client might want to set a progress bar to some value instead of printing it with {}.

In fact, not even multirust-rs will use it, because I'm planning to use Terminal::delete_line and Terminal::carriage_return in the implementation.

But if it's okay, then I will just provide "fake" Display implementations for it to satisfy the constraints.

@Diggsey
Copy link
Contributor

Diggsey commented Feb 25, 2016

Thanks for implementing this!

@Diggsey Diggsey closed this as completed Feb 25, 2016
@Diggsey
Copy link
Contributor

Diggsey commented Feb 25, 2016

Actually, I'll leave this open to track the missing "time remaining" feature.

@Diggsey Diggsey reopened this Feb 25, 2016
@crumblingstatue
Copy link
Contributor Author

I'm working on

@crumblingstatue
Copy link
Contributor Author

(Stupid touch devices and their inaccuracies)
I'm working on this.
I'm going to use the time crate, since we already implicitly depend on it

@crumblingstatue
Copy link
Contributor Author

We should try to resolve #76 before closing this issue.

@vks
Copy link

vks commented Apr 5, 2016

I think this can be closed. #76 is orthogonal.

@Diggsey Diggsey closed this as completed May 8, 2016
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

No branches or pull requests

3 participants