Add MultiBar for safe deferred progress bar configuration by jgrund · Pull Request #787 · console-rs/indicatif

jgrund · 2026-04-03T21:55:35Z

Summary

Implements the design from #677: introduces MultiBar, a builder type that captures progress bar configuration without triggering any draws. This prevents the common footgun where a ProgressBar draws to stderr before being added to a MultiProgress, corrupting the terminal.

MultiBar — plain data struct with with_* builder methods mirroring ProgressBar. No draws until added to MultiProgress.
MultiProgressInput — #[doc(hidden)] #[non_exhaustive] enum with From<ProgressBar> and From<MultiBar>, enabling backwards-compatible impl Into<MultiProgressInput> on all MultiProgress::add/insert methods.
Materialization — when added to MultiProgress, creates a hidden ProgressBar, applies config via with_* (no draws), sets the remote draw target, then starts steady tick last.

Before (footgun)

let pb = ProgressBar::new(100);
pb.set_message("downloading"); // draws to stderr before mp knows about it!
pb.enable_steady_tick(Duration::from_millis(100)); // spawns thread drawing to stderr!
mp.add(pb); // too late, screen already corrupted

After (safe)

let pb = mp.add(
    MultiBar::new(100)
        .with_message("downloading")
        .with_steady_tick(Duration::from_millis(100))
);
// pb is a fully configured ProgressBar, safely managed by mp

Existing code using mp.add(ProgressBar::new(...)) continues to work unchanged.

🤖 Generated with Claude Code

djc · 2026-04-04T08:48:37Z

I think the core of this idea is still good. I'm not convinced we want to make semver-compatible changes to do it, and I'm not sure I want to invest much time in reviewing a PR that it looks like is mostly LLM-generated without much human oversight. Separately, not sure if the name is right since the general concept seems applicable to other ways of building up a ProgressBar, too.

jgrund · 2026-04-06T19:52:07Z

Hi,

I think the core of this idea is still good. I'm not convinced we want to make semver-compatible changes to do it.

Do you want to make API breaking changes instead here?

I'm not sure I want to invest much time in reviewing a PR that it looks like is mostly LLM-generated without much human oversight.

Yes, this code was mostly generated with LLM, but I designed the overall solution and am manually reviewing it / testing it / updating it etc...

Separately, not sure if the name is right since the general concept seems applicable to other ways of building up a ProgressBar, too.

Would you prefer this become a builder / default API for ProgressBar overall?

djc · 2026-04-07T12:02:38Z

I think the core of this idea is still good. I'm not convinced we want to make semver-compatible changes to do it.

Do you want to make API breaking changes instead here?

Sorry, I mistyped. I don't want to make semver-incompatible changes for this.

I'm not sure I want to invest much time in reviewing a PR that it looks like is mostly LLM-generated without much human oversight.

Yes, this code was mostly generated with LLM, but I designed the overall solution and am manually reviewing it / testing it / updating it etc...

Okay, that seems fine. Suggest you write the PR description by hand in the future.

Separately, not sure if the name is right since the general concept seems applicable to other ways of building up a ProgressBar, too.

Would you prefer this become a builder / default API for ProgressBar overall?

Yes, I think that might be better.

jgrund · 2026-04-07T21:31:02Z

Yes, I think that might be better.

Looking over this a bit, would there be any functional difference between a builder and the

ProgressBar::with_* API (if we fill in the bits that don't currently have a ProgressBar::set_* equivalent)?

I.E. we could have a ProgressBarBuilder (which would be more idiomatic) but that then seems redundant against the existing with_* methods.

One thing I could think of:

Create ProgressBarBuilder
Have the with_* methods be marked as deprecated
Create a MultiProgress::add_bar method (which only takes builder)
Have the MultiProgress::add method be marked as deprecated

That keeps things semver compatible and gives a clear migration path to users. Thoughts?

djc · 2026-04-08T08:50:54Z

One thing I could think of:

Create ProgressBarBuilder

Have the with_* methods be marked as deprecated

Create a MultiProgress::add_bar method (which only takes builder)

Have the MultiProgress::add method be marked as deprecated

That keeps things semver compatible and gives a clear migration path to users. Thoughts?

Given the prevalence of the old with_ methods and the lack of utility for anyone not using MultiProgress I'd prefer to avoid explicit deprecation of the old with_ methods. But otherwise your plan sounds good to me, especially deprecating MultiProgress::add() in favor of a new method that adds the builder directly (not sure add_builder() is the best name though).

Introduces MultiBar, a builder type that captures progress bar configuration without triggering draws. This prevents the common footgun where a ProgressBar draws to stderr before being added to a MultiProgress, corrupting the terminal (see console-rs#677). MultiProgress::add/insert methods now accept both ProgressBar (backwards compatible) and MultiBar via impl Into<MultiProgressInput>.

Addresses review feedback on console-rs#787. - Rename MultiBar -> ProgressBarBuilder for broader applicability. - Add MultiProgress::{register, register_at, register_from_back, register_before, register_after}, each taking ProgressBarBuilder. - Deprecate MultiProgress::{add, insert, insert_from_back, insert_before, insert_after}, pointing at the register* equivalents. - Remove the MultiProgressInput enum (no longer needed now that the new API takes ProgressBarBuilder directly). - Split ProgressBarBuilder::materialize into a panic-safe build_unregistered that runs before a MultiState slot is reserved, preventing slot leaks on panic. - Add regression tests for with_steady_tick and the tab_width+style interaction. - Migrate examples and render tests to the new API.

jgrund · 2026-04-09T19:52:56Z

FYI: Latest push adds ProgressBarBuilder and MultiProgress::register{,_at,_from_back,_before,_after} as the way to use it. Previous ways are marked as deprecated.

The intra-doc link fails the `rustdoc::private_intra_doc_links` lint because `set_for_stderr` is `pub(crate)`. Drop the hyperlink and describe the behavior in prose.

djc · 2026-04-10T13:38:56Z

+        pb
+    }
+
+    fn internalize_builder(


Nit: let's call this internalize().

djc · 2026-04-10T13:39:32Z

@@ -0,0 +1,436 @@
+use std::borrow::Cow;


Let's call this module builder.

djc · 2026-04-10T13:39:52Z

+}
+
+// ProgressStyle doesn't implement Debug, so we print all other fields
+impl fmt::Debug for ProgressBarBuilder {


Nit: this should go below the inherent impl block.

djc · 2026-04-10T13:40:23Z

+// ProgressStyle doesn't implement Debug, so we print all other fields
+impl fmt::Debug for ProgressBarBuilder {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("ProgressBarBuilder")


Nit: use let Self { .. } = self; to make sure we keep track of all the fields.

djc · 2026-04-10T13:42:28Z

+}
+
+impl ProgressBarBuilder {
+    fn base() -> Self {


This should be Default impl instead (presumably it can be derived).

djc · 2026-04-10T13:43:41Z

+        let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden());
+        let pb = mp.register(ProgressBarBuilder::no_length());
+        assert_eq!(pb.length(), None);
+    }


This looks like a lot of tests for a fairly shallow API wrapper. Please exercise some judgement in which tests add value/coverage.

djc · 2026-04-10T13:44:35Z

 }

+#[test]
+fn builder_with_steady_tick() {


Why do we need a separate test for this?

djc · 2026-04-10T13:44:50Z

 }

+#[test]
+fn builder_tab_width_propagates_to_style() {


Why do we need a separate test for this?

djc · 2026-04-10T13:45:35Z

+    /// [`MultiProgress`]. See [#677] for details.
+    ///
+    /// [#677]: https://github.com/console-rs/indicatif/issues/677
+    pub fn register(&self, builder: ProgressBarBuilder) -> ProgressBar {


Not sure I love the register() name/prefix. How about using push() and/or insert() like the std collections types?

@chris-laplante thoughts?

What about make, make_after, make_before, etc.? or alternatively, realize, realize_after, and so on,

Those don't really make sense to me? The question is what relation makes sense between a MultiProgress and a ProgressBar (builder). The MultiProgress doesn't make a ProgressBar, right, nor does it really realize it?

The thought was that a MultiProgress makes a ProgressBar from a builder. Or alternatively, a builder is 'realized' to produce the ProgressBar.

Maybe it makes sense to use build, build_after, etc.:

let builder = ProgressBarBuilder::/* whatever */; let pb = mp.build(builder); // or let pb = mp.build_after(builder, &existing_pb);

Ah, I understand what're you getting at now. I think I'd still prefer to emphasize the relation between the MultiProgress and the resulting ProgressBar over the "realization" from builder to ProgressBar.

I see - the issue is that we already have insert and friends (https://docs.rs/indicatif/latest/indicatif/struct.MultiProgress.html#method.insert_after). For the builder flavors, what about insert_with, insert_after_with, etc.?

Yes, I had to choose a new verb as all the others were overloaded with some other meaning (even _with has a specific connotation in this codebase).

Let me know what you land on and I'll make the change.

jgrund force-pushed the jgrund/multi-bar branch 5 times, most recently from 8a4d376 to 9eea25c Compare April 3, 2026 22:57

jgrund force-pushed the jgrund/multi-bar branch from 9eea25c to 84e78ba Compare April 6, 2026 19:52

jgrund added 2 commits April 8, 2026 14:12

jgrund force-pushed the jgrund/multi-bar branch from 84e78ba to 9d90838 Compare April 8, 2026 18:13

Fix rustdoc link to private ProgressStyle::set_for_stderr

330ea31

The intra-doc link fails the `rustdoc::private_intra_doc_links` lint because `set_for_stderr` is `pub(crate)`. Drop the hyperlink and describe the behavior in prose.

djc reviewed Apr 10, 2026

View reviewed changes

Conversation

jgrund commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before (footgun)

After (safe)

Uh oh!

djc commented Apr 4, 2026

Uh oh!

jgrund commented Apr 6, 2026

Uh oh!

djc commented Apr 7, 2026

Uh oh!

jgrund commented Apr 7, 2026

Uh oh!

djc commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jgrund commented Apr 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-laplante Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-laplante Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jgrund commented Apr 3, 2026 •

edited

Loading

djc commented Apr 8, 2026 •

edited

Loading

chris-laplante Apr 10, 2026 •

edited

Loading

chris-laplante Apr 10, 2026 •

edited

Loading