[Feature] Vertical alignment

[jaskij]

I'm honestly impressed by comfy-table, and it handled a multiline string like a champ. What I would like to ask is for the ability to set vertical alignment (perhaps through a new enum and method, perhaps through adding new values to CellAlignment).

For example, in the table from this screenshot below, I would like the single line text in the header to be in the lower line, so that unit, status and state transition are all in the same line.

[Nukesor]

Thanks :)

I see how this can be useful. It should also be rather easy to implement.
The code that's responsible for filling up lines can be found over here:
https://github.com/Nukesor/comfy-table/blob/main/src/utils/formatting/content_format.rs#L130

I guess it would make sense to simply reuse the CellAlignment enum. However, one thing I'm not sure about though is the naming of the functions:

Right now we have Column::set_cell_alignment and Cell::set_alignment. It would probably make sense to rename those to set_cell_horizontal_alignment and Cell::set_horizontal_alignment? Having a set_alignment besides a set_vertical_alignment is kind of confusing :D.

I'm also thinking about where to add these functions. To Row, Column or possibly both (In case both are allowed, which of them takes precedence?)?

I usually opt for Column, as the style used to format data is usually determined by its type, i.e. the Column. In the case of a header however, we have a special kind of row that behaves differently than the rest of the table.

Would it be acceptible to set the alignment for the individual Cells instead of the full header row in the case of your example?

[jaskij]

I guess it would make sense to simply reuse the CellAlignment enum.

I wonder if it could be made into a bit flag without breaking the API. You could then do something like CellAlignment::Left | CellAlignment::Top to make CellAlignment::TopLeft. It's not critical, but it's useful and simplifies the number of cases. There is the enumflags2 crate which seems made for this purpose.

Right now we have Column::set_cell_alignment and Cell::set_alignment. It would probably make sense to rename those to set_cell_horizontal_alignment and Cell::set_horizontal_alignment? Having a set_alignment besides a set_vertical_alignment is kind of confusing :D.

With bit flags, or just all options in one enum, it would stay the same call, without renaming the function. So, hopefully, existing code would function as-is.

I'm also thinking about where to add these functions. To Row, Column or possibly both (In case both are allowed, which of them takes precedence?)?

I usually opt for Column, as the style used to format data is usually determined by its type, i.e. the Column. In the case of a header however, we have a special kind of row that behaves differently than the rest of the table.

Would it be acceptible to set the alignment for the individual Cells instead of the full header row in the case of your example?

I'd say at least Column and Cell, with row possibly too. IMO the order of precedence should be, from least to most important, Column, Row, Cell. My reasoning for this is going from least to most specific. As you said, the column typically denotes the data type, then there are special rows (header and summary/footer, typically), and then someone might want to make a specific cell special for whatever reason.

Speaking of headers - it would probably need to be a separate issue, but it would be nice to have them more generalized as well. Like a summary (think financial tables) or a header column - say, when you do a comparison table or something.

[Nukesor]

I really like the idea approach via enumflags2. I've seen this approach before, but I've never really worked with it.

How would you handle something like CellAlignment::Top | CellAlignment::Left | CellAlignment::Right? Would that be a `TopCenter?

I.e.

Left | Right => Centered text at top
Left | Right | Bottom => Centered text at bottom

Top | Bottom => Left aligned text at the middle (vertically) of the cell
Top | Bottom | Left | Right => Center Centered text at the middle (both vertically and horizontally) of the cell

I guess this would result in a breaking change, as the current CellAlignment::Center would then translate to Top | Bottom | Left | Right instead of Left | Right. This wouldn't be too much of a problem though as we're on a v6 release candidate anyway.

It would also probably make sense to add a new helper enum CellAlignment::CenteredText which is a combination of Left | Right?

[jaskij]

For reference, here's how Qt does it.

I did some quick experimentation with enumflags2 and actually can't come up with an idea which:

• wouldn't expose the BitFlags<> type as part of interface
• didn't require an amount of boilerplate that makes enumflags2 obsolete

Partly because enumflags2 requires that each value in the enum only has one bit set.

How would you handle something like CellAlignment::Top | CellAlignment::Left | CellAlignment::Right? Would that be a `TopCenter?

I.e.

Left | Right => Centered text at top Left | Right | Bottom => Centered text at bottom

Top | Bottom => Left aligned text at the middle (vertically) of the cell Top | Bottom | Left | Right => Center Centered text at the middle (both vertically and horizontally) of the cell

For clarity, I'll be using HCenter for horizontal center and VCenter for vertical.

I believe that setting one flag, as well as both of them (for center), is obvious.

The only question here is how to handle when no flags for the axis are set. IE if neither Left nor Right is set, do you use the current default (whatever it is), or do you use center? Using bits, for the horizontal direction, we have:

• 0b11 - HCenter
• 0b10 - Left
• 0b01 - Right
• 0b00 - the question: Left or HCenter?

There are good arguments both way - using Left means less breakage, using HCenter means the API is more self-consistent. The choice is really up to you, whatever you do you will need to explain it in the docs anyway.

(The same goes for vertical, skipped for the sake of brevity).

[Nukesor]

You're most likely no longer using this library, but here's my answer 😓
Just go ahead and build it like you imagined it, if you're still interested :D

Your proposal sounded straight forward and well thought out, so I guess you'll come up with a good implementation as well.

Proper testing would be needed though!

[jaskij]

Frankly, I barely touched Rust over the past year. So there's that. And I don't remember anything related to this issue, sadly. Am just now getting back to coding in Rust at work, but that's an entirely different thing. Iirc I do use comfy-tables in a few places though.

FWIW, I'm pretty sure the screenshot in OP was taken from this repo: https://github.com/jaskij/systemd-unit-status