search

muesli4 / table-layout

Layout data in grids and pretty tables. Provides a lot of tools to get the cell formatting right (positional alignment, alignment on specific characters and limiting of cell width)..
BSD 3-Clause "New" or "Revised" License
38 stars 11 forks source link

Generalise grid, gridLines, and tableLines to produce StringBuilders. #24

Closed Xitian9 closed 3 years ago

Xitian9 commented 3 years ago

Instead of Strings.

gridString and tableString have been left producing Strings because they have ‘String’ in their names, however it would also be reasonable to generalise these.

muesli4 commented 3 years ago

The versions ending in String were more a conceptual string but not fixed to the concrete type. My initial concern for not turning this yet into StringBuilder was that I did not want users to force manually specializing. But perhaps writing :: String is acceptable. Thus, I would ask you to also convert those.

I'm willing to break the API for version 1.0.0.0. Perhaps we can get some other changes in to make it worth the effort. :)

Xitian9 commented 3 years ago

I'm happy to do that if you want.

In hledger we've used the convention of having a main builder whose name ends with B, and a String-specialised version without the B. I don't think it would be a big issue to keep the legacy name for the specialised version, and have gridBuilder and tableBuilder for the generalised versions, if you wanted that.

Xitian9 commented 3 years ago

I've made the changes you requested. Let me know if you'd like any further.

muesli4 commented 3 years ago

What I would probably do is provide a special version of every function with a B suffix. I like the idea. In general, I don't know which of the functions are actually used. But it is not too much effort for me to maintain and keep them, so I left it the way it is.

Xitian9 commented 3 years ago

What I would probably do is provide a special version of every function with a B suffix. I like the idea. In general, I don't know which of the functions are actually used. But it is not too much effort for me to maintain and keep them, so I left it the way it is.

Which functions do you mean by ‘every function’? Do you just mean gridString and tableString, or do you also mean grid, gridLines, and tableLines?

muesli4 commented 3 years ago

Please use imperative in commit messages. Otherwise, seems fine to me, thanks a lot.

Xitian9 commented 3 years ago

Commit message updated.

muesli4 commented 3 years ago

I resolved the conflicts locally and rebased manually.