magic-wormhole / magic-wormhole.rs

Rust implementation of Magic Wormhole, with new features and enhancements
European Union Public License 1.2
722 stars 78 forks source link

[transfer] Give progress information #98

Closed piegamesde closed 3 years ago

piegamesde commented 3 years ago

In order for the CLI to display a fancy progress bar, the transfer API needs to expose such information. Together with progress, we probably want to track transfer speed.

One possibly way to implement this is to pass some callback to the file transfer that calls the method once something happens. The alternative way would be to spawn the file transfer (or the UI updating) in a new task and let them send events over a channel. Note that the latter can be implemented using the former outside of the API (by having the callback writing into a channel).

The difficulty I see here is to know when the caller should be informed. After every packet? Every percent? Every second? Probably some combination of multiple. Can the transfer expect the UI updating to be reasonably quick? Otherwise we a forced to make this multiple tasks.

Prior art/related work:


The actual CLI implementation is secondary to this issue. Of course, it make sense to develop both together in order to test the ergonomics of the API, but we can always make the output pretty later on. There are plenty of fancy progress bar crates out there :)

brightly-salty commented 3 years ago

Just wanted to put indicatif out there as a possible crate to help with this. You can provide it a callback and an optional theme and it will do the behind-the-scenes. https://docs.rs/indicatif/0.15.0/indicatif/

moaz-mokhtar commented 3 years ago

This is 2nd time for me to contribute in a Rust project. Can I do a try? I did a research and found below:

  1. progress-streams: have errors when implementing the sample code in the doc. That is why I preferred to check alternatives.
  2. pbr: I found this crate which defined as: Console progress bar for Rust Inspired from pb, support and tested on MacOS, Linux and Windows.
    • Sample: image
piegamesde commented 3 years ago

Sure, go ahead :)

I haven't tried progress-streams myself so I can't judge on it. Have you already found any alternatives?

I'd like to keep the scope of this issue small and mainly focus on the library part of it. For now, a few print statements on the application side will be fine, just enough to see that it's working.

moaz-mokhtar commented 3 years ago

The alternate will be pbr.

Regarding the scope, I understand that I have to do below:

Is that correct? if something different define me the requirements.

piegamesde commented 3 years ago

Go ahead.

crd477 commented 3 years ago

I also just learned about Linya. It claims to "stick to Rust basics and offers a smaller feature set than alternatives, but is very efficient and also intuitive to use". This one might also be a good option.

moaz-mokhtar commented 3 years ago

I also just learned about Linya. It claims to "stick to Rust basics and offers a smaller feature set than alternatives, but is very efficient and also intuitive to use". This one might also be a good option.

I did read about that yesterday. I don't have info which is better pbr or Linya.

moaz-mokhtar commented 3 years ago

I did create a branch named "progress_bar" but I didn't commit or push. Progress bar implemented when sending data as below screenshot: image

For receiving files progress, I tried to implement it but I have some errors which I'm fixing now.

moaz-mokhtar commented 3 years ago

Thank Almighty Allah... I did it.

Receive print screen: image

Send print screen image

====================== Notes:

  1. I didn't commit or push yet, so seeking your guide.
  2. After file sent and received, progress bar can be replaced with a message as below code.
    pb.finish_print("Receive done");

bp: is the PorgressBar struct. finish_print() a function to show a message under impl of ProgressBar.

piegamesde commented 3 years ago

Great, many thanks. You can simply put all of your changes in one commit and open a pull request so that we can all try it out and have a look at the code.

piegamesde commented 3 years ago

Implemented in #116.