-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cell redesign proposal #58
Comments
Hi ! By the way, regarding the cons around the Enum-based solution, I think we could implement the |
Did I get it right that by "a variant for generic trait objects" you meant something like Ah, and macros in all 3 cases can work without modifying external API. \o/ |
Yes,that's what I meant, but I'm agree it's not really elegant. I've been thinking on this on the way back to home, and maybe we can mix your 3 proposals into 1, and let the library user choose. Regarding your first proposal, the plan is to define something like struct Table<T: CellContent> { ... } right ? Table<Vec<String>>
Table<Table<Vec<String>>
// An so on ... As you said, we would have to spread the type accross the entire API, but I'm not fully agree when you say we would be locked to one single type. In fact we should be able to write something like impl <T> CellContent for Box<T> where T: CellContent + ?Sized { ... }
// Then use
Table<Box<CellContent>> This would enable statically AND dynamically dispatched trait-based solution. Merging together your proposals 1 an 2. We can also hide it behind a type like pub type BoxTable = Table<Box<CellContent>>; And finally, your third proposal can also join the party since the enum can also implement |
Wow, combining the good parts of different solutions to get the best one. I feel proud for being treated like that, thank you! 😄 Now that I think of it, most of the pros and cons I've shown stem from the question "How to deliver the desired feature with breaking API as less as possible, ideally not breaking at all, at the same time trying not to sacrifice raw performance?" So far I only considered dynamic dispatch + enum, because it can be done implicitly albeit giving up on effectiveness. But if genericity is worth going all out, then I'm on it! |
Current implementation progress:
|
Hi ! Sorry for the late reply, I've been quite busy this week. I havent had time yet to start experimenting around this redesign. I'll give it a look ASAP. In the meantime could you share your code ? |
The code is here: https://github.com/hcpl/prettytable-rs/tree/cell-redesign. |
This is a proposal to remake
Cell
and related functionality to simplify fixing issues #45, #46 and possibly #47 (since this proposal, if accepted, will allow defining custom behaviour of cell elements in case of the outer table being shrinked).Here are some ways how this proposal can be implemented.
1. Statically dispatched trait-based
Pros:
Cell
s, for other structs they're either avoidable, or have to do something with formatting and styling.cell!
macros can remain the same.Cons:
String
. If this is the case this mode works perfectly fine)Table
's andRow
's backing storage is a homogeneousVec
, the<T>
parameter would be needed to spread across the whole API.2. Dynamically dispatched trait-based
Pros:
Flexible: users will be able to define new types with their own behaviours in the table.
Easy to adapt existing public API:
Cell
constructors: types can be backwards-compatibly generalized, whereas the amount of constructors remains the same.cell!
macro wouldn't have to be modified at all.As a result, public API changes would be minimal and backwards-compatible.
Cons:
Cell
has to storeCellContent
boxed since we wouldn't know the exact type. This would certainly have performance implications, though their extent varies among usages.CellContent
implementations.3. Enum-based
Pros:
Cons:
CellContentBox
and add implementation into all related functions and methods, which is a more cumbersome process than implementing a trait. It also makes code less clean and readable.Cell
constructors would probably have constructors for every enum variant.cell_*!
for every enum variant.UPD: More info about macros and wording.
The text was updated successfully, but these errors were encountered: