ajalt / mordant

Multiplatform text styling for Kotlin command-line applications
https://ajalt.github.io/mordant/
Apache License 2.0
935 stars 33 forks source link

Convert ProgressAnimation to multiplatform #149

Closed aSemy closed 6 months ago

aSemy commented 6 months ago

This PR converts ProgressAnimation to a multiplatform implementation.

This is a breaking change (because it now requires coroutines to use a ProgressAnimation), and not necessarily the only solution (See https://github.com/ajalt/mordant/issues/148#issue-2075120668), so I'm only opening at as draft. However, I thought it might be useful to share!

Summary

ajalt commented 6 months ago

Thank you for the contribution! I'm actually already working on a complete rewrite of the progress code in the multi-progress branch which includes multiplatform support via coroutines as well as keeping JVM threads as an option.

So I'll close this PR in favor of that work, but if you're able, I'd love feedback on that branch.

For multiplatform coroutines, you would use the new mordant-coroutines module to do something like this:

val animation = progressBarLayout {
    progressBar()
}.animateInCoroutine(terminal)
val task = animation.addTask(total = 10)
launch { animation.run() }
while(!animation.finished) {
    delay(500)
    task.advance(1)
}
CharlyRien commented 6 months ago

Hello ajalt,

I'm using your new API and it's looking great. But I currently have an issue using it, everything I print after the following runBlocking with terminal.println("something") is printed before the progressBar or it's more like the progressBar is reprinted after the println, do you know from where it could come?

runBlocking {
            val progress = progressBarContextLayout<BigDecimal> {
                spinner(Spinner.Dots(TextColors.brightBlue))
                text { "Match percentage found so far $context%" }
                progressBar()
                percentage()
            }.animateOnThread(terminal)

            val task = progress.addTask(
                context = 0.toBigDecimal(),
                total = 100,
                completed = 0
            )

            progress.execute()

            val bestCombination = Algorithm
                .run() // <---- returns a Flow here 
                .buffer(CONFLATED)
                .onEach {
                    task.update {
                        context = it.matchPercentage
                        completed = it.progressPercentage.toLong()
                    }
                    delay(1000L)
                }
                .onCompletion {
                    task.update { completed = 100 }
                    progress.removeTask(task)
                }.last()
                .individual
                .also {
                    terminal.println(
                        """Research result:
                         ....
                        """) // <---- Here the progress bar is printed also after that
                  }      
ajalt commented 6 months ago

@CharlyRien Thank you for the feedback!

That's a fundamental limitation of terminal animations. We have to move the cursor to redraw the next frame, and if you print below the animation we won't know where to move the cursor to (or the animation might not even be on screen anymore!).

If you really need the progress bar above other content, you'll have to make one big layout and animate that manually. Right now it would look something like this:

val progress = progressBarContextLayout<BigDecimal> {
    ptext { "Match percentage found so far $context%" }
}.cache()
val now = TimeSource.Monotonic.markNow()
val animation = terminal.animation<Context> {
    verticalLayout {
        cell(progress.build(ProgressState(it.percent, it.total, it.completed, now, it.status)))
        cell(it.textToShow)
    }
}

launch {
    while(true) {
        animation.update(Context(percent, textToShow, total, completed, now, status))
        delay(progress.refreshPeriod)
    }
}

But I'll think about ways to make that easier.

CharlyRien commented 6 months ago

Thanks for your answer and the piece of code.

For my use case I am not only doing that progress bar, once the progress is done I need to continue doing things. (like printing the results of the algorithm for example) So yes it is pretty much "mandatory", at least for my use case, to not have the progress bar being printed after each text.

If it can give you any idea of implementation, I am currently using another library for the progressbar and it's something like that:


val progressBar = ProgressBarBuilder()
        .hideEta()
        .setTaskName("Best build research")
        .setInitialMax(100)
        .setStyle(ProgressBarStyle.UNICODE_BLOCK)
        .build()

     val bestCombination = progressBar.use { progressBar -> Algorithm
                .run()
                .buffer(CONFLATED)
                .onEach {
                    progressBar.setExtraMessage("${it.matchPercentage}% match found so far")
                    progressBar.stepTo(it.progressPercentage.toLong())
                    delay(1000L)
                }
                .onCompletion {
                    progressBar.stepTo(progressBar.max)
                }.last()
                .individual
                .also {
                   println(
                        """Research result:
                         ....
                        """) 
                  }  
          }

It is working pretty well but it makes me use another library than clikt. (Also the progress bar animation is less "sexy" in the library I currently use: https://github.com/ctongfei/progressbar)

aSemy commented 6 months ago

Thank you for the contribution! I'm actually already working on a complete rewrite of the progress code in the multi-progress branch which includes multiplatform support via coroutines as well as keeping JVM threads as an option.

So I'll close this PR in favor of that work, but if you're able, I'd love feedback on that branch.

Thanks @ajalt! I've taken a look and it's a little hard to view the changes, and I'm not a coroutines expert so it's a little hard for me to judge. Would you mind making a draft PR?

Does the multi-progress branch have a SNAPSHOT release I can try out?

I looked into adding coroutines support a little bit and I realised it's quite a fundamental change, so I'm pleased that you're already making the effort!

ajalt commented 6 months ago

@aSemy I created the draft PR #151, so we can move this discussion over there @CharlyRien Thanks for the use case. I pushed an update that will allow you to print below the progress animation normally once it's finished. Or you can stop it early with the new stop() or clear() methods. I also added a clearWhenFinished parameter to animateOn* to remove the bar entirely once it's done.