Positioned Progress Tasks - Before or After Other Tasks

[thomhurst]

Morning, I raised an issue about inserting Progress Tasks into specific positions. My use case is I'm doing stuff asynchronously, and I want to be able to insert tasks before/after other tasks, but I may not immediately know about them. And currently everything just gets appended to the end.

The issue was #1249 .

I thought it'd be easier if I took this on myself. The change is fairly trivial, since it's backed by a standard List<>, so we just need to open up some public methods that allow inserting into the list at specific indexes.

If you want to see this in action, I wrote a small console application to test it's working as expected.

Console code:

using Spectre.Console;

var random = new Random();

await AnsiConsole.Progress()
    .Columns(new TaskDescriptionColumn(), new ProgressBarColumn(), new PercentageColumn(), new ElapsedTimeColumn(), new RemainingTimeColumn(), new SpinnerColumn())
    .StartAsync(async ctx =>
    {
        var foo1 = ctx.AddTask("foo1");
        var foo2 = ctx.AddTask("foo2");
        var foo3 = ctx.AddTask("foo3");

        var tasks = new List<ProgressTask> { foo1, foo2, foo3 };

        _ = Task.Delay(1300).ContinueWith(t =>
        {
            var afterFoo1 = ctx.AddTaskAfter("afterFoo1", foo1);
            tasks.Add(afterFoo1);
        });
        
        _ = Task.Delay(1900).ContinueWith(t =>
        {
            var beforeFoo3 = ctx.AddTaskBefore("beforeFoo3", foo3);
            tasks.Add(beforeFoo3);
        });

        while (!ctx.IsFinished)
        {
            foreach (var progressTask in tasks)
            {
                if (!progressTask.IsFinished)
                {
                    progressTask.Increment(random.Next(1, 5));
                }
            }

            await Task.Delay(500);
        }
    });

Output:

      foo1 ---------------------------------------- 100% 00:00:25 00:00:00
 afterFoo1 ---------------------------------------- 100% 00:00:24 00:00:00
      foo2 ---------------------------------------- 100% 00:00:25 00:00:00
beforeFoo3 ---------------------------------------- 100% 00:00:23 00:00:00
      foo3 ---------------------------------------- 100% 00:00:25 00:00:00

[thomhurst]

Hi @patriksvensson any chance at a quick look?

[patriksvensson]

Currently on vacation, but perhaps someone in @spectreconsole/maintainers have some spare cycles before I get back

[thomhurst]

@patriksvensson doesn't look like anyone else is actively looking at this repository. I haven't seen issues or pull requests decrease in a couple of months. Is there anyone I can tag to get their attention?

[patriksvensson]

You can always tag @spectreconsole/maintainers, but it won't go any faster since we all watch notifications.

We're doing this in our spare time for no pay, and some periods are more busier than others.

[thomhurst]

You can always tag @spectreconsole/maintainers, but it won't go any faster since we all watch notifications.

We're doing this in our spare time for no pay, and some periods are more busier than others.

Ah okay. Sorry didn't mean to be a pain. Will be patiently waiting for when you guys have time then :)

[FrankRay78]

It's no pain @thomhurst, and just to say this repo is alive and well. For me, school holidays and not enough hours in the day mean my involvement dwindles currently. We appreciate the PR and your efforts on this, it's just the cycle times are longer than your typical corporate software development.

[thomhurst]

Hi again @spectreconsole/maintainers. Would anyone have any time soon?
Thanks

[thomhurst]

@patriksvensson @FrankRay78 would anyone be able to take a look? It's a very small change, just exposing methods that manipulate the inner list.

[FrankRay78]

It's not strictly my area of maintainership, but I am empowered to review and merge all going well. As I hate to see good contributions languishing, I will assign this to myself and make time over the next week to review.

[FrankRay78]

I haven't forgotton @thomhurst, I've got a busy couple of days but it's assigned to myself and I will review this.

[FrankRay78]

Hi @thomhurst. Firstly, thank you for a high-quality PR, really glad to have reviewed it, and thanks for being so patient with it. I'd most certainly welcome more contributions like this, and I would make time to review them promptly if you tag me in. I normally maintain the CLI sub-section, but am branching out.

Secondly, my review seems to have actually committed to your your branch 😱, see above, this commit: 0fe55e1

I like to treat PR reviews as a collaborative endeavour, rather than assuming I know more than the contributor (which very often is not the case). I have not merged the changes yet, as I would like you to cast your eyes over what I've suggested - in the mistaken commit. It's style over substance, but I felt the changes made the code more consistent across all the various AddTask methods. If you agree, then great, let me know and I will merge. If you don't, then equally fine, just outline why you feel that way, and we can always revert my commit or do something else.

Anyhow, great work and much appreciated.

[thomhurst]

Thanks @FrankRay78 , really appreciate it!

Just reviewed your changes and I'm perfectly happy with them. I originally just called overloads and obviously couldn't double lock, but I agree that your approach is more consistent and probably less prone to bugs if people extend that class in the future. 😄

[FrankRay78]

I'll merge this tomorrow. Welcome to being a spectre.console contributor.