spectreconsole / spectre.console

A .NET library that makes it easier to create beautiful console applications.
https://spectreconsole.net
MIT License
9.17k stars 472 forks source link

Progress formatting options for `PercentageColumn` #1277

Open Tyrrrz opened 1 year ago

Tyrrrz commented 1 year ago

Is your feature request related to a problem? Please describe.

I want to show a progress table using spectre. It works really well as it is, however, some operations take a really long time to complete, so I would like to be able to show more detailed progress values (e.g. 50.41% instead of just 50%).

The code responsible for this seems to be here:

https://github.com/spectreconsole/spectre.console/blob/813a53cdfaf24fb0ad759cc7af74d10b197c6618/src/Spectre.Console/Live/Progress/Columns/PercentageColumn.cs#L19-L24

And it doesn't provide any outside entry points to customize this behavior.

Describe the solution you'd like

New ProgressColumn property of type string that serves as the format for task.Percentage.ToString(...). Potentially also a IFormattingProvider property too, but I personally need to use the current culture formatting anyway.

Describe alternatives you've considered

I could create my own ProgressColumn but I'd end up re-writing most of the stuff in the PercentageColumn.

Additional context

None that I could think of.


Please upvote :+1: this issue if you are interested in it.

Tyrrrz commented 1 year ago

One challenge that I just found with this, is that the internals of ProgressBar and related classes normalize the progress values to integers in the range of 0 to 100. The fractional part of the progress seems to be lost here:

https://github.com/spectreconsole/spectre.console/blob/813a53cdfaf24fb0ad759cc7af74d10b197c6618/src/Spectre.Console/Live/Progress/ProgressTask.cs#L225-L230

Which means just providing the formatting options to PercentageColumn is not enough on its own.

patriksvensson commented 1 year ago

@Tyrrrz Hmm, sorry this is my first day back after vacation, so I'm probably wrong about this, but how is the fraction lost?

private double GetPercentage() 
{ 
    var percentage = (53.23 / 150M) * 100M; // 35,48666666666667
    percentage = Math.Min(100, Math.Max(0, percentage)); // 35,48666666666667
    return percentage; // 35,48666666666667
} 

Or am I missing something?

Tyrrrz commented 1 year ago

@Tyrrrz Hmm, sorry this is my first day back after vacation, so I'm probably wrong about this, but how is the fraction lost?

private double GetPercentage() 
{ 
    var percentage = (53.23 / 150M) * 100M; // 35,48666666666667
    percentage = Math.Min(100, Math.Max(0, percentage)); // 35,48666666666667
    return percentage; // 35,48666666666667
} 

Or am I missing something?

Sorry, @patriksvensson, I think you're right! I was following the code roughly in the debugger and I think I made the wrong conclusion that the percentages are rounded earlier than I thought. Please ignore my last comment.

patriksvensson commented 1 year ago

So, I've been thinking, and perhaps we can add a setting to the column whether it should round it to nearest integer or not, and with how many decimals.

Tyrrrz commented 1 year ago

So, I've been thinking, and perhaps we can add a setting to the column whether it should round it to nearest integer or not, and with how many decimals.

That would do it too. I'm curious, however, why are you leaning toward this option versus just providing a format string, which would take care of all this for you? Is it because you want to limit acceptable formats to just those that represent percentages?

patriksvensson commented 1 year ago

@Tyrrrz Format strings are hard 😄, and I want to keep the number of support issues down if I can. We could however (in addition) add a new column where you get a lambda with the task, and where you return a string of your choice. Perhaps that would be a middle road?

Tyrrrz commented 1 year ago

@Tyrrrz Format strings are hard 😄, and I want to keep the number of support issues down if I can. We could however (in addition) add a new column where you get a lambda with the task, and where you return a string of your choice. Perhaps that would be a middle road?

For my use case, being able to specify just the decimal places is enough. I'm just thinking that maybe there's merit to providing a more general solution for formatting the string. A lambda would work too 🙂