Open Nukesor opened 8 months ago
I gave this a lot of thought and here's what I came up with.
The current implementation doesn't allow:
Since this is about rethinking how the API would work and since this change will be breaking anyway, let's use this opportunity and do it properly. My proposal for a new API design would look like this.
Accessing columns is currently annoying and error prone. One needs to know the index of the column, which isn't ergonomic at all. My proposal would be to move more logic into the column, which is why we need to make it more accessible.
Column
struct will gain a name
in addition to its index.Table
.
-> Column
will get [set_]header
functions.Table
will get a [set_]show_headers
function to en/disable displaying of column headers.Table
will get a get_column_by_name()
getter, which may return columns by their identifier.table.add_column(name: &str) -> Result<&mut Column>
and a table.add_columns(columns: Vec<String>) -> HashMap<String, &mut Column>
function.Column
instances to the table.
Having a &mut Column
returned should make things easier though.I found no good way of actually removing columns. The problem with column removal is, that it needs to implicitely remove the respective cells of all current rows as well. Without that, we wouldn't know which cells to display or not.
As the whole idea of this request is to remove implicit logic, I decided to forbid removal of columns.
Instead of removing columns, it should be made more convenient to hide
them, which is basically the same as a temporary "soft" removal.
Currently, the functionality to hide a column is implemented as a Constraint
, which is a bit dirty.
It's not necessarily a constraint and having it as a constraint toggling the column in a program annoying, since the old constraint needs to be restored as soon as the column is made visible again.
hidden
field to the Column in addition to respective hide(bool)
/hidden()
getter/setter functions.ColumnConstraint::Hidden
variant.Previously, comfy-table was able to do things in an "optimistic" manner. It was just given some strings and from there on it tried to display things to the best of its abilities.
With the trend to giving developers more power, we also have to introduce error handling.
ComfyError
class, depending on the needed complexity introduce thiserror
.Result<T, ComfyError>
.DuplicateColumn
.
All functions need to document which errors might be returned.There were two additional requested features:
With the current builder design, these features won't be implemented, as this would only lead to confusion. It would allow scenarios like this:
- Add some rows
- Swap some column positions
- Add some new rows in the original layout
-> The columns of the new rows are now in the wrong order.
There're two solutions to this problem:
Make rows named as well.
Vec
s and only HashMap
s instead.This solution makes the whole library harder to use for a very small userbase.
Actually use an builder pattern.
TableBuilder
in which one may specify columns, rows and their content.builder.build()
, which returns a Table
.That Table
now only allows styling related actions, content may no longer be added/removed etc.
This solution doesn't really solve the problem, as people also want to add content to the table after they've already built it once.
To be honest, I'm inclined to tell people that they should just rebuild the whole table if they want complex interactions. \ Comfy-table is designed to be a minimalistic table builder library and shouldn't be used as an interactive table renderer.
-> Until there's a good design proposal that doesn't make the library less usable/unusable for the vast majority of its users, these two features won't be added.
I am very happy with the current state of comfy-table and I don't need any of said functionality myself. Even though I designed and specified the new architecture, I won't implement it. Also, if you find any design flaws, please point them out and let's try find a better solution.
As usual, I'll give any MRs thorough reviews as long as they're well documented and properly tested (via actual tests).
So, if anyone needs this and has some spare time at their hand, I would be happy to see this merged into comfy-table :).
The current column handling is rather convenient, as columns get populated as rows and cells are added to the table.
However, this logic is rather implicit. There's no way to remove or add columns half-way through a table creation process.
To fix this
I'm actually in favor of the second approach, since the goal of comfy-table is to be minimalistic and convenient to use. Explicit column handling is more of an edge-case and the 99% use case is covered by implicit column creation.
However, it could be very tricky to design the API around an optionally explicit column handling, since such APIs tend to get confusing quite quickly.
This ticket is mostly for discussion on how such an API would look like.
Relevant tickets/MRs: https://github.com/Nukesor/comfy-table/pull/121 Previous ticket: https://github.com/Nukesor/comfy-table/issues/124