tj / terminal-table

Ruby ASCII Table Generator, simple and feature rich.
MIT License
1.53k stars 121 forks source link

Unicode support and bugfixes #113

Closed nanobowers closed 3 years ago

nanobowers commented 3 years ago

@tj and @nateberkopec

I added support for unicode type tables in this PR, while trying to maintain backward compatiblity with the API and existing tests. There are very subtle formatting differences (removal of certain intersections near colspans) that happened in order to support unicode, but other than that, existing tests should be intact.

In addition to supporting a variety of unicode border styles, this PR also fixes:

I realize this a lot to drag into one PR, but most of the fixes for the above issues somewhat naturally fell out of the change in how the internals of how styling was handled.

nateberkopec commented 3 years ago

Commenting to mark that I've seen this and will get to it this week.

nateberkopec commented 3 years ago

A few comments - also, please write up your changes in History.rdoc. Don't worry about headings, just describe the changes.

This is really great work, the amount of hours you must have put into this! Thank you so much.

I'm thinking this should probably be a major version bump due to the changes to the class hierarchies, although it looks like you managed to avoid removing or significantly changing any actual method names or APIs. What do you think?

nanobowers commented 3 years ago

@nateberkopec , Thanks for the nice comments and reviewing such a large PR. I have addressed the unnecessary returns and updated the History.rdoc (without incrementing the version, so will need another tweak on based on the decision regarding version number)

I agree that this is a pretty big change (re: class hierarchies), and also it changes output slightly for the Ascii case, so gems that use this gem may end up breaking their test suites if they expect the old output. That said, despite minor changes to ascii output formatting, the API itself didn't break from release to release. I did consider making slightly more aggressive changes that would break backward compatibility, but since this is such an old and established gem, it didn't feel appropriate.

I also have considered adding support for a footer which would affect the Style/Border API (currently we only support a header), addressing a few more enhancement tickets and cleaning up the code a bit more.

Anyway, I'm fine with a 2.1.0 or a 3.0.0 or whatever you recommend, but if the decision is to do a major rev, I'd want to add footer support before doing so.

nateberkopec commented 3 years ago

@nanobowers With the age of this gem, I think we should just be cautious and make it a 3.0.

Do you want to add on footer support here or can I merge?

nanobowers commented 3 years ago

Please go ahead and merge, I'll try to submit another PR for footer support soon. I just committed updates to rev the version-number to 3.0.0 in the History.rdoc and version.rb.

nanobowers commented 3 years ago

@nateberkopec Actually, can you hold off on merging this for another few days? i think there's a better way to do it to make the footer /separator features more transparent.

nateberkopec commented 3 years ago

Can do 👍

nanobowers commented 3 years ago

@nateberkopec I think this is a better implementation - it removed some of the embedded logic and moves it to table elaboration. As part of this change, I exposed a way for users to choose different separators, and provided some more styling options on the separators. Additional tests / documentation have been added. Apologies for the churn on this.

nateberkopec commented 3 years ago

Really incredible work! This is more attention than terminal-table has gotten in years! Thank you so much @nanobowers

nateberkopec commented 3 years ago

3.0 will release tomorrow

nateberkopec commented 3 years ago

@nanobowers thanks for all the linking/reminders to close