vbauerster / mpb

multi progress bar for Go cli applications
The Unlicense
2.31k stars 123 forks source link

add ability to manually refresh rather than refresh rate timer #27

Closed coryb closed 6 years ago

coryb commented 6 years ago

Hi Vladimir, thanks for your library, it is very interesting! It took me a few hours to really understand how you were doing it, but it is a very nice design!

This PR will allow for manually refreshing the bars, instead of just using timer based. My use-case is that I am monitoring something via subprocess and I want to update the progress bars only when I get information from the subprocess. To do this I added a manual p.Refresh() function and I use it in conjunction with a really long refresh rate, something like mpb.WithRefreshRate(time.Hour).

The main reason I want to only refresh when necessary is that my tool records stdout/stderr to a log file so that I can later debug user sessions, and I want to minimize the progress bar updates so the log file does not become excessively long.

I was thinking about a way to disable the automatic refresh rate, but I wanted to get your thoughts before I put any more effort into this. I am fine with the time.Hour refresh rate hack, but if you are interested in the PR then I can look into some approaches.

Thank you! -Cory

codecov-io commented 6 years ago

Codecov Report

Merging #27 into master will increase coverage by 0.2%. The diff coverage is 88.46%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #27     +/-   ##
=========================================
+ Coverage   68.34%   68.55%   +0.2%     
=========================================
  Files           8        8             
  Lines         496      512     +16     
=========================================
+ Hits          339      351     +12     
- Misses        131      134      +3     
- Partials       26       27      +1
Impacted Files Coverage Δ
progress.go 73.88% <75%> (+0.07%) :arrow_up:
progress_posix.go 63.63% <94.44%> (+2.52%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e17ac3d...c7ba554. Read the comment docs.

vbauerster commented 6 years ago

@coryb Thank you for contribution and idea. This is really valid use case, and why not? I've reviewed your PR and come to conclusion that it is better to let user provide their refresh channel. That way it is easy to disable internal auto refresh ticker and no need to set long refresh rate hack, actually I've implemented this in manual-rr branch. So I encourage you to check out and test if it works for your needs.

coryb commented 6 years ago

@vbauerster thanks for the reply, sorry for the delay, too busy :) I have verified that your manual-rr branch works for my use-case. I agree, it is a better design. Thank you!

vbauerster commented 6 years ago

https://github.com/vbauerster/mpb/pull/28 merged to master.