Skip to content
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

The cell width should be calculated using the data when applying a formatter #2

Closed
koffeinfrei opened this issue Nov 26, 2021 · 4 comments · Fixed by #3
Closed

The cell width should be calculated using the data when applying a formatter #2

koffeinfrei opened this issue Nov 26, 2021 · 4 comments · Fixed by #3

Comments

@koffeinfrei
Copy link
Contributor

The documentation

formatter: a Proc to format the cells, while conserving the inferred alignment from the body contents. Default to "->(n : CellType) { n.to_s }"

suggests that the value from the fomatter does not influence the calculation / alignment of the cell.

When using a formatter to output the value with a color (using Crystal's Colorize) the cell width is not calculated based on the data value, but based on the value provided by the formatter.

Tablo::Table.new(table_data, connectors: Tablo::CONNECTORS_SINGLE_ROUNDED) do |t|
  t.add_column("Diff", width: COLUMN_WIDTH, formatter: ->(n : Tablo::CellType) { n.colorize(:red).to_s }) { |n| n[1] }
end

Expected output:
image

Actual output:
image

Tabulo also mentions this:

In most terminals, if you want to print text that is coloured, or has certain other styles such as underlining, you need to use ANSI escape sequences, either directly, or by means of a library such as Rainbow that uses them internally. Tabulo needs to properly account for escape sequences when performing the width calculations required to render tables. The styler option on the add_column method is intended to facilitate this.

Is this the intended behaviour for tablo as well? If so the width calculation should be based on the data value instead of the formatted value.

@matt-harvey
Copy link

matt-harvey commented Nov 26, 2021

Hi @koffeinfrei , author of Tabulo here.

Just to clarify, Tabulo has two distinct concepts in this space: the formatter; and the styler. These are configurable as distinct options when initializing a table column.

The formatter's output is taken into account for the width calculations. For example, a formatter for displaying dollar amounts might turn an integer 1 into a string $1.00, and then the table's internals must take the width of that string into account when rendering the table.

From what I can see, this behaviour around the formatter option is the same in Tablo.

while conserving the inferred alignment from the body contents

I gather this is also the same as in Tabulo, where numbers are right-aligned by default, and strings are left-aligned by default, and the formatter option doesn't affect whether something is regarded as a number or a string for the purpose of determining its default left/right/centre alignment. I believe that's what this documentation comment is referring to—only that.

Let's now consider the case where one wants to make that string $1.00 appear red in the terminal. In Tabulo, one would then use the styler option to do so; and any additional control characters in the styler's output, would not be taken into account for the width calculations. This enables colours etc. to be applied without breaking the rendered table layout.

Prior to v1.5.0, Tabulo did not have a distinct styler option, meaning that it was not possible to apply ANSI sequences to render colours etc. without breaking the width calculations and hence the table layout. The point of styler is to provide a mechanism to manipulate these styles without breaking the formatting, hence addressing the issue you've raised here.

As far as I can see, Tablo does not have a similar "styler" mechanism; though perhaps I am wrong, as I am not too familiar with this library.

@koffeinfrei
Copy link
Contributor Author

koffeinfrei commented Nov 27, 2021

👋 @matt-harvey Thanks for your thorough explanation!
So yes basically tablo is missing the styler concept. @hutou Is this something we could add? Are there other plans? Or should the expected behaviour deviate from tabulo?
In the end my use case is to have colored output. If there are any other ways to achive this please let me know.

@hutou
Copy link
Owner

hutou commented Nov 27, 2021

Hi,
Tablo is an adaptation of tabulo to the Crystal language made almost 3 years ago, and has not evolved much since then. I made this adaptation while learning Crystal and there is more than likely a lot of room for improvement to make Tablo as flexible as Tabulo.

So, in its current version, Tablo does not have a style option. This feature is definitely very useful and I think that when I find time to work on the Tablo project again, I will put it on the top of the 'to do' pile!

In the meantime, if necessary, it is always possible to work around the missing style option by using a trick, such as the (ugly, but working) code below:

require "tablo"
require "colorize"

data = [
  [1, 2, 3],
  [-4, 5, 6],
  [7, -8, -9],
]

table = Tablo::Table.new(data) do |t|
  t.add_column("Col1") { |n| n[0] }
  t.add_column("Col2") { |n| n[1] }
  t.add_column("Col3") { |n| n[2] }
end

table.each do |row|
  cols = row.to_s.split("|")
  r = cols.map_with_index { |c, i|
    c =~ /-\d+/ && i == 2 ? c.colorize(:red) : c
  }.join("|")
  puts r
end
puts table.horizontal_rule(Tablo::TLine::Bot)

test_cr

@koffeinfrei koffeinfrei mentioned this issue Nov 27, 2021
1 task
@koffeinfrei
Copy link
Contributor Author

@hutou I had a quick go at the feature in the meantime: #3. Let me know what you think.

@hutou hutou closed this as completed in #3 Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants