olekukonko / tablewriter

ASCII table in golang
MIT License
4.28k stars 359 forks source link

Add support for Unicode box drawing characters #207

Closed gagern closed 1 year ago

gagern commented 1 year ago

The ASCII lines can be hard to read under some circumstances. Using Unicode box drawing characters should on most terminals ensure that adjacent glyphs actually connect, leading to a smoother perception where lines and table content can be visually distinguished more easily.

craigbox commented 1 year ago

CP 437 says hello, and thank you.
(I'm very keen to see this merged.)

aaabramov commented 1 year ago

Any chance this could be merged sometime?

olekukonko commented 1 year ago

@gagern I want to apologise for the very late response.

  1. can we have more test
  2. What if there are new set symbols, We may have some we support, but is it possible to also introduce flexibility in such a way a user can add a custom symbol table

Regards

gagern commented 1 year ago
  1. can we have more test

Sure. Anything specifically you'd like to see tested?

I've added some more test cases for basic functionality, including new support for footers which required some more work to get right. I also added Unicode variants of some existing test cases, even if in some of these the Unicode rendering isn't quite optimal yet. I added comments in these situations.

  1. What if there are new set symbols, We may have some we support, but is it possible to also introduce flexibility in such a way a user can add a custom symbol table

Personally I would recommend letting the implementation gain a bit more maturity before opening it up further. The reason being that as long as this is fresh, there is a high chance things will move a bit so that any API added would become a burden to development. Once this has been stable for a bit, and particularly if there is concrete requests, then opening it up makes sense.

I'm also not sure what the most reasonable API would be. Would it make sense to expose the predefined order of characters, which currently is an implementation detail? Should we have one function invocation per character, with each call e.g. providing current symbol and the desired replacement? Or should we expose our symbolID constants and allow using them as keys in a map?

olekukonko commented 1 year ago

Can you take a look at https://github.com/olekukonko/tablewriter/pull/115, it allows for flexible customisation can this theme be incorporated in your proposal or did i merge too early ?

gagern commented 1 year ago

Thanks for merging. I hadn't noticed #115 before. In its current form my approach doesn't distinguish between e.g. a horizontal line at the boundary of the table and a horizontal line in the inside. It's certainly doable to add that.

I'm not convinced the approach from #115 for specifying these characters is the best either, though. Say I have a theme where the outer boundary is a double line but the inner lines are single. Then for merged cells I would ideally want to use characters such as as indicated in my AutoMergeRowsUnicode test case. Granted, I'm not doing that yet either, but it would be possible. So if we devise a new representation for a set of characters, I would like to include that information.

I feel validated in my approach to not make the character set manipulations public yet. That way, we can add more functionality without breaking the API introduced here. We might internally go for something like #115 for now, or we might add some more names to my symbolID enumeration, but either way I would suggest keep the representation internal and only expose a few pre-made themes at this stage.

I'll think some more, see whether I can come up with a naming scheme which is both intuitive and expressive enough, then write a change if I find the time. If someone wants to beat me to it, feel free to. It would make sense to update all the Unicode tests to use the double boundary single inner line theme by default, since that would expose mismatches more readily.

To keep things exciting, we might want to support something like this eventually:

╔════════════╤═══════════╤═════════╗
║ FIRST NAME │ LAST NAME │   SSN   ║
╟────────────┼───────────┼─────────╢
║ John       │ Barry     │ 123456  ║
╟┄┄┄┄┄┄┄┄┄┄┄┄┼┄┄┄┄┄┄┄┄┄┄┄┼┄┄┄┄┄┄┄┄┄╢
║ Kathy      │ Smith     │ 687987  ║
╟┄┄┄┄┄┄┄┄┄┄┄┄┼┄┄┄┄┄┄┄┄┄┄┄┼┄┄┄┄┄┄┄┄┄╢
║ Bob        │ McCornick │ 3979870 ║
╚════════════╧═══════════╧═════════╝
craigbox commented 1 year ago

@gagern Or this 😉

olekukonko commented 1 year ago

@gagern Interesting, when should we be expecting PR :)

olekukonko commented 1 year ago

@craigbox do you mind sharing more light?

craigbox commented 1 year ago

(I was just making a joke referring to the regular use of line-drawing characters in MS-DOS.)