ProgressBar interface #47

jzelinskie · 2016-04-11T19:50:43Z

In order to encapsulate the logic around ProgressBars, we should have a ProgressBar interface.

For example, in the main package we could detect the interactivity of the shell once and use a no-op ProgressBar implementation to avoid that logic bleeding throughout the program. A real implementation of this interface would also cover the default behaviors like setting the MaxWidth and what completed lines look like, etc...

The text was updated successfully, but these errors were encountered:

philips · 2016-04-20T04:55:20Z

@jzelinskie can't we just use the progress bar from rkt?

cc @jonboulle

jzelinskie · 2016-04-20T16:22:11Z

@philips does that handle drawing multiple bars on the screen at once? I didn't think it did.

jonboulle · 2016-04-20T16:23:28Z

@dgonyeo is working on that right now, looping him in

On Wed, Apr 20, 2016 at 6:22 PM, Jimmy Zelinskie notifications@github.com
wrote:

@philips https://github.com/philips does that handle drawing multiple
bars on the screen at once? I didn't think it did.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#47 (comment)

jzelinskie · 2016-04-20T16:24:38Z

That's cool, either way I think the code can be much cleaner if we use an interface to minimize the ProgressBar logic that leaks into other functions.

cgonyeo · 2016-04-20T16:47:21Z

My redesign right now is actually aiming to do that, since (as I'm sure you've noticed) using ioprogress in its current state requires the user to write a good amount of logic.

The current API (feedback would be appreciated!):

type MultilineProgress struct {
    // DisplayWidth can be set to influence how large the progress bars are.
    // The bars will be scaled to attempt to produce lines of this number of
    // characters, but characters of different lengths may still be printed.
    // When this is set to 0 (aka unset), 80 character columns are assumed.
    DisplayWidth int

    // Has unexported fields.
}
    MultilineProgress will manage the copying of io.Readers to io.Writers while
    printing multiline progress bars about the progress of each copy.


func (mp *MultilineProgress) DisplayAndWait(displayTo io.Writer, drawInterval time.Duration) error
    DisplayAndWait will wait until all registered readers have finished copying
    (reached EOF), while drawing a progress bar for each reader to displayTo
    that updates every drawInterval. If displayTo is a terminal, draw codes will
    be used to draw over each previous progress bar. If it is not, the current
    copied amount and total will be printed for each reader.

func (mp *MultilineProgress) RegisterReader(reader io.Reader, name string, size int64, dest io.Writer)
    RegisterReader registeres an io.Reader with this MultilineProgress, and
    copies all data from reader into dest until EOF. name and size are used to
    display a progress bar. When size is zero, the size is assumed to be
    unknown.

Example usage would be something like this:

mp := &ioprogress.MultilineProgress{}
mp.RegisterReader(res1.Body, "ACI", num1, file1)
mp.RegisterReader(res2.Body, "Signature", num2, file2)
mp.DisplayAndWait(os.Stderr, time.Second)

jzelinskie · 2016-04-20T17:06:19Z

Our usage actually isn't based around Readers; has that been taken into consideration as well?

cgonyeo · 2016-04-20T17:13:47Z

Not at all, right now it's geared for just docker2acis use case. Would you just want to manually update the progress bars?

On Apr 20, 2016, at 13:06, Jimmy Zelinskie notifications@github.com wrote:

Our usage actually isn't based around Readers: has that been taken into consideration as well?

―
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

jzelinskie · 2016-04-20T17:20:26Z

Yeah, we manually update our progress bars: https://github.com/coreos/quayctl/blob/a75bf650cd08f91c036b92c92e6060c6d8be96e2/engine/torrent.go#L75-L160

cgonyeo · 2016-04-20T17:27:09Z

I can probably split out the progress bar logic and have the io.Reader copy logic just use that. I'll keep you posted.

cgonyeo · 2016-04-20T20:47:27Z

@jzelinskie Would this work for you giuys? coreos/ioprogress#7

jzelinskie · 2016-04-20T21:09:37Z

@dgonyeo I think all we need to be able to specify prefixes and suffixes for each progress bar, set the total and the current state, and the frequency of redraws.

cgonyeo · 2016-04-20T21:17:04Z

The ProgressBarPrinter struct lets you specify prefixes and suffixes (and update them) for each progress bar and set the current state (from 0 to 1). For the frequency of redraws, you'll need to write your own logic for that. This should be an example of how to do that: https://github.com/dgonyeo/ioprogress/blob/master/iocopy.go

If a wrapper around ProgressBarPrinter that has the concept of current/total values and that has the logic for continually redrawing would be useful, it shouldn't be hard to implement.

jzelinskie · 2016-04-20T21:54:22Z

That wrapper would be perfect for our use case.

cgonyeo · 2016-04-20T22:57:33Z

I'll implement that in a followup PR.

On Apr 20, 2016, at 17:54, Jimmy Zelinskie notifications@github.com wrote:

That wrapper would be perfect for our use case.

―
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

philips added area/usability low hanging fruit kind/enhancement labels Apr 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ProgressBar interface #47

ProgressBar interface #47

ProgressBar interface #47

ProgressBar interface #47

Comments