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

Be more flexible in the column values that are accepted #1617

Closed stephanstapel closed 1 week ago

stephanstapel commented 3 weeks ago

Is your feature request related to a problem? Please describe. I was wondering why you limit the columns values for AddRow() to string objects: https://github.com/spectreconsole/spectre.console/blob/2cc6c457ade3c0b795ebb4d0f8868e1befc56a6c/src/Spectre.Console/Extensions/TableExtensions.cs#L148 Is there a design idea behind this why this isn't an object that is then analyzed for it's real type and rendered in the appropriate way?

Describe the solution you'd like Eventually accept param object[] as a parameter. Might need to add reflection or invoke .ToString() straight away.

Additional context I am happy to send in a pull request, I'd just like to confirm that this idea is not violating your design ideas.


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

patriksvensson commented 3 weeks ago

Yes, the design idea is that it only works with strings, so the user must choose how they want their values to be rendered in the table.

I would like to keep this unopinionated in the library itself, but it shouldn't be much work adding a method in your application that accomplishes this.

public static class MyTableExtensions
{
    public static Table AddRow(this Table table, params object[] columns) 
    {
        table.AddRow(columns.Select(x => x.ToString().ToArray));
    }
}
stephanstapel commented 3 weeks ago

ok, an extension method would also work, thanks for the idea. Could you elaborate why adding the function that you just mentioned shouldn't be part of the component?

patriksvensson commented 1 week ago

@stephanstapel because then I (the @spectreconsole/maintainers team) will have to decide how certain values are formatted, and someone is not going to like it. Object.ToString doesn't take a CultureInfo instance, so it will require extra plumbing and setup. Having a simple string API forces the API consumer to format their values, and if they do it a lot, it's trivial to implement an extension method that does what they want.

FrankRay78 commented 1 week ago

Irrespective, thanks for caring enough to raise the issue and offer your time for a PR @stephanstapel Appreciated.