ProgressSpinners with tick strings skips the last frame, panics on one frame

[grahamc]

With this spinner configuration we see 1, 2, then 3. 4 is never printed.

let spinner = ProgressBar::new_spinner();
spinner.enable_steady_tick(Duration::from_millis(1000));
spinner.set_style(
    ProgressStyle::with_template("{msg}{spinner}")?.tick_strings(&["1", "2", "3", "4"]),
);

With this configuration we get a panic:

let spinner = ProgressBar::new_spinner();
spinner.enable_steady_tick(Duration::from_millis(1000));
spinner.set_style(ProgressStyle::with_template("{msg}{spinner}")?.tick_strings(&["1"]));

The application panicked (crashed).
Message:  attempt to calculate the remainder with a divisor of zero
Location: /home/grahamc/.cargo/registry/src/github.com-1ecc6299db9ec823/indicatif-0.17.0/src/style.rs:166

[chris-laplante]

For the first issue, could I see the full context?

The second issue looks like an easy fix. Thanks for the report!

[cole-h]

The first issue can be replicated with this:

use indicatif::{ProgressBar, ProgressStyle};
use std::time::Duration;

fn main() {
    let spinner = ProgressBar::new_spinner();
    spinner.enable_steady_tick(Duration::from_millis(1000));
    spinner.set_style(
        ProgressStyle::with_template("{msg}{spinner}")
            .unwrap()
            .tick_strings(&["1", "2", "3", "4"]),
    );

    std::thread::sleep_ms(10_000);

    spinner.finish_and_clear();
}

[chris-laplante]

The way tick_strings (and tick_chars) currently works is that the last string is used for the "finished" state. Only the first n - 1 strings/chars are used when the spinner is actually running. If you change that last call to be finish() you will see that the 4 is printed. So potentially this is just an issue of documentation.

With that in mind, this should actually be rejected by tick_strings:

spinner.set_style(ProgressStyle::with_template("{msg}{spinner}")?.tick_strings(&["1"]));

@djc what do you think?

[cole-h]

In that case, would it be better to provide a separate function that will set this "finished" state, instead of taking the last of the tick chars/strings as the "finished" state?

Maybe even as an argument to the finish family of functions... (Though I realize that would be an API break.)

[chris-laplante]

In that case, would it be better to provide a separate function that will set this "finished" state, instead of taking the last of the tick chars/strings as the "finished" state?

Maybe. I do agree the current API is a little confusing.

Maybe even as an argument to the finish family of functions... (Though I realize that would be an API break.)

That's the hard part :/

[cole-h]

I'll leave the API design to people who are actually good at that (i.e. not me), but maybe finish_char / finish_string (or maybe better: final_*) would be an "OK" design.

[djc]

I agree that the API is unclear. We could potentially add a new with_tick_strings(&[&str], &str) API and deprecate the old one? I don't think we want to release a semver-incompatible version so soon after 0.17.

[MarijnS95]

Yes please, that seems like a neat API. We ran into this (last tick string not printed) as well and set out to PR a documentation update before finding this issue.