[Request for comment; not ready for merge]: add insert_after / insert_before to MultiProgress

[chris-laplante]

This greatly simplifies the code. It also clarifies the API very nicely:
the old MultiProgress::insert method is not practically useful since the
indices do not necessarilly correspond to visual order, except for
specific usage patterns.

The ability to insert after/before particular progress bars is much more
useful.

[chris-laplante]

This pull request leverages a new crate I developed (https://github.com/Agilent/generational_token_list). I developed it specifically for this use-case.

I realize it is a breaking API change, but I think it is beneficial since the previous insert API doesn't really make sense (since indices do not necessarily correspond to visual order).

Still to do here:

• Update examples
• Documentation

[djc]

I'm on the fence about using your crate. Basically, this seems to exchange the use of indices (with panics at run-time if we get it wrong) for something more efficient that relies on unsafe code. Since our use of this functionality seems fairly limited, I'm not sure it makes sense to use something like your crate over just using indexes. We could wrap the current code in a separate type to make it easier to verify.

Even if your crate was very well tested (with comprehensive test coverage including Miri) and audited by someone conversant in unsafe, I'm not sure I'd be up for doing this here.

[mostthingsweb]

The only use of unsafe is in GenerationalTokenList::iter_mut, which this crate isn't using. I'd be happy to gate the unsafe code behind an "unsafe" feature (and in fact I'll probably do that anyway).

[chris-laplante]

So I've been struggling to rewrite the examples that use MultiProgress in terms of the new API I am proposing. Before I continue on the path, let me ask: is there a chance of this pull request getting accepted anyway? Otherwise I won't spend the time. I truly believe MultiProgress would be more useful if it worked via relative positioning of progress bars, i.e. the insert_after or insert_before. The existing examples are just not practical - no one wants to have to keep track of indices - I feel the library should do that for the user.

Actually, I think I can implement insert_after and insert_before without having to use the generational_token_list. The recently added insert_from_back is instructive.

[djc]

That would be great! I do agree the insert_after()/insert_before() design makes a lot more sense, but taking on a new/immature extra dependency to do it feels like not quite worth it.

[chris-laplante]

That would be great! I do agree the insert_after()/insert_before() design makes a lot more sense, but taking on a new/immature extra dependency to do it feels like not quite worth it.

OK great :). The only problem I see with the current design (which is what initially inspired me to replace the guts) is if someone tries to insert_after a progress bar that hasn't been drawn yet, there is no way to figure out the ordering. This is because MultiProgress::push sets the draw_state[idx] to None. Any ideas?

Ah, I think something like this will work actually:

pub fn insert_after(&self, after: &ProgressBar, pb: ProgressBar) -> ProgressBar {
        let mut state = self.state.write().unwrap();

        let idx = after.state.lock().unwrap().draw_target.remote().unwrap().1;

        let ordering = state.ordering.iter().position(|i| i == idx).unwrap();
...

[chris-laplante]

See #331 instead

[chris-laplante]

@djc Do you think there's any chance I could reconstitute this changeset (i.e. using [generational_token_list](https://github.com/Agilent/generational_token_list)). It would alleviate #419 and also make my life with #426 easier. I think I'd be able to do it without introducing the unsafe portion of the code.

[djc]

Let's discuss this in a new issue? Can you layout the core problem and why the generational list is your preferred way to solve it?