Skip to content

Make ProgressBar::index public#724

Open
strowk wants to merge 1 commit intoconsole-rs:mainfrom
strowk:main
Open

Make ProgressBar::index public#724
strowk wants to merge 1 commit intoconsole-rs:mainfrom
strowk:main

Conversation

@strowk
Copy link

@strowk strowk commented Jul 27, 2025

This could be useful for some user crates, for example potentially can help solve the issue with order of child progress bars that exists in this crate - https://github.com/emersonford/tracing-indicatif/blob/main/src/pb_manager.rs#L217-L221

@chris-laplante
Copy link
Collaborator

It's been a while since I looked at MultiProgress in depth, but IIRC I thought the index was sort of an implementation detail? We go through some trouble to maintain a separate ordering, which kind of implies that index isn't exactly an index: https://github.com/console-rs/indicatif/blob/main/src/multi.rs#L399. But I could be wrong...

@strowk
Copy link
Author

strowk commented Aug 1, 2025

Thanks for your response, @chris-laplante .

Let me give you a bit of background why I thought this might be useful.

There is a crate that I have found convenient to use - https://github.com/emersonford/tracing-indicatif .
It allows to reduce some boilerplate around managing progress bars. It does have, however, one bug, that I demonstrate in this test - strowk/tracing-indicatif@cf0cb0a#diff-a0262fea108af44a98cc49e5eb72641963dd816513f2d0a22eb738fcfed55ebeR161-R181

Without access to index() method, this test would fail and results would look like this:

foo{}
--> child2{}
--> child1{}

I.e the order of progress bars would be wrong (child 2 would be before child 1). This is a known issue in this crate, which I have managed to fix (test passes in that linked revision) by using custom version of indicatif (from which this PR is created) and by calling index() - strowk/tracing-indicatif@cf0cb0a#diff-a646224308d682586cd49b1953007c405ab373195b6e3c08ed2e1ae0cbe17a52R239-R241

Now, as you can see in that bit above, this works as a combination of index() and insert:

let parent_index = parent_pb.index().unwrap_or(0);
self.mp.insert(
    parent_index + siblings_offset,
    pb_span_ctx.progress_bar.take().unwrap(),
)

, which in theory could've work like this:

self.mp.insert_after_with_offset(
    parent_pb,
    siblings_offset,
    pb_span_ctx.progress_bar.take().unwrap(),
)

, given if indicatif supported a method insert_after_with_offset, that would be similar to this -

indicatif/src/multi.rs

Lines 140 to 142 in e1f410d

pub fn insert_after(&self, after: &ProgressBar, pb: ProgressBar) -> ProgressBar {
self.internalize(InsertLocation::After(after.index().unwrap()), pb)
}

, but also take offset to add to index. This could be alternative approach that would also fix the issue in tracing-indicatif.

Now, I do not actually know whether author of tracing-indicatif would agree with either of those honestly :)
The author of that crate did note that tracking index of progress bars is not trivial in this comment - emersonford/tracing-indicatif#21 (comment)
I have found that the index is already available in indicatif and thought maybe it can be utilized and it did work in practice. I wouldn't claim that the tests I have done are exhaustive enough to say that it works in absolutely all cases, but the tool for which I needed this, hasn't been showing any weird order of progress bars in those several days that I use it (about dozen times a day).

I also can in principle live without either of these being fixed in upstreams, my forks of both indicatif and indicatif-tracing kind of work for me at the moment. So you are free to close this as well, no big.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants