Show current worker goroutine status below progress bar

[mrnugget]

GIF:

TODO:

• Timestamp needs to not be truncated, but I'm not sure where to best put the timestamp.

[mrnugget]

Timestamps truncated and moved to the right. TODOs checked off. Ready for the big review @sourcegraph/campaigns!

[mrnugget]

Note to reviewers: progressWithStatusbarsTTY and progressWithSatusBarsSimple have a lot of similarities with progressTTY and progressSimple respectively. I've tried to extract some of the common stuff and embed progressTTY in progressWithStatusBarsTTY. That helped but not as much as it would've in a language where inheritance was a first class concept.

The problem in Go is this:

type inner struct {
	name string
}

func (i *inner) String() string { return i.format() }
func (i *inner) format() string { return fmt.Sprintf("my name is %s", i.name) }

type outer struct {
	*inner
}

func (o *outer) format() string { return "i am outer!" }

func main() {
	i := &inner{name: "inner"}
	fmt.Println(i)

	o := &outer{inner: i}
	fmt.Println(o)
}

That prints:

my name is inner
my name is inner

See this StackOverflow answer for details. Short version: Go doesn't have virtual methods.

I might try to "invert" the code and have the structs' methods call functions, passing themselves in, but I'm not sure it's worth it.

Because right now I don't think the duplication is a problem. The code that is duplicated depends on the callee's state and doesn't contain a lot of duplicated logic. The hairy bits are not duplicated.

[eseliger]

Looks good! Thanks for taking on the effort of designing a TUI 👍

[eseliger]

is this todo comment related to the newly added line? If not, I'd add a newline here to avoid confusion

[mrnugget]

I'll let @LawnGnome decide. He added the TODO and I wasn't sure whether he had something else in mind, like

TotalSteps int
CurrentStep int

or something like that.

[LawnGnome]

That was my intention, but I think it's unnecessary now with the work that you've done since I wrote that, so we could remove the TODO.

[chrispine]

LGTM, thanks!

[LawnGnome]

This LGTM! I think there's a lot of good stuff in here.

There is an issue right now with outputting to a non-TTY, so I'm going to mark this as changes requested, but I think there's a clear path to fixing it and getting this landed.

I also tested this on Windows. Behold:

[LawnGnome]

That was my intention, but I think it's unnecessary now with the work that you've done since I wrote that, so we could remove the TODO.

[LawnGnome]

This doesn't actually return the simple version, which means that ANSI escape codes are still used and the output (if being redirected to a file) is unusable.

However, progressWithStatusBarsSimple doesn't implement ProgressWithStatusBars at the moment, so that's another issue that would have to be dealt with.

I've pushed 19b6ec5 with what I think would be a reasonable fix, but haven't merged it into this branch: PTAL!