console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.22k stars 238 forks source link

Duration becomes 0 after calling `finish()` #557

Open suomela opened 1 year ago

suomela commented 1 year ago

If I have got a ProgressBar with ProgressStyle that uses a template like "… {elapsed}/{duration} …". As usual, I repeatedly call .inc(…) and finally .finish(). However, I am typically getting this kind of printout:

That is, the estimated total duration makes plenty of sense, except that after calling finish it drops to 0. I would expect that duration would reflect the real final duration when the progress bar is finished.

I guess the problem is around lines 292–297 of src/state.rs:

pub fn duration(&self) -> Duration {
    if self.len.is_none() || self.is_finished() {
        return Duration::new(0, 0);
    }
    self.started.elapsed() + self.eta()
}

It seems to be explicitly returning 0 when is_finished(). Is this intentional?

djc commented 1 year ago

"Intentional" is a big word. :smile: I suppose we don't ever store the time when we finished, so we don't really have any better value we can show here. I suppose we could add an Instant to the Done variants of Status?

suomela commented 1 year ago

@djc To me it seems that elapsed works well both before and after finish, and eta is explicitly set to 0 after finish, so if we just removed the part || self.is_finished() in line 293 and let it fall back to elapsed + eta, shouldn't it behave better?

I tried it out here and it seems to work: https://github.com/suomela/indicatif — there's examples/elapsed-duration.rs for playing with this, and a one-line fix attempt in this commit.

djc commented 11 months ago

It might work well immediately after finish, but if a redraw happens sometime after finishing, then elapsed will continue to move forward, which I don't think makes sense.

suomela commented 11 months ago

I see, I think {elapsed} then has the same issue with redraws?

djc commented 11 months ago

I was talking about elapsed. I haven't actually tried but based on the code I think that would be the effect.