Add a :formatter option to Kino.DataTable.new/2 #441
Conversation
Make Kino.DataTable.value_to_string/2 private again Make default formatter be &value_to_string/2 (no module)
lib/kino/data_table.ex
Outdated
| def update(kino, tabular, opts \\ []) do | ||
| {data_rows, data_columns, count, inspected} = prepare_data(tabular, opts) | ||
| Kino.Table.update(kino, {data_rows, data_columns, count, inspected}) | ||
| formatter = Keyword.get(opts, :formatter, &value_to_string/2) | ||
| Kino.Table.update(kino, {data_rows, data_columns, count, inspected, formatter}) |
There was a problem hiding this comment.
update is rather designed for updating data and I wouldn't expect formatter to change after the table is created. So let's support it only in new :)
lib/kino/data_table.ex
Outdated
| * `:formatter` - a 2-arity function that is used to format the data | ||
| in the table. The first parameter passed is the `key` (column name) and | ||
| the second is the value to be formatted. When formatting column headings | ||
| the key is the special value `:__header__`. Defaults to the builtin | ||
| formatter. |
There was a problem hiding this comment.
I would change the function to return {:ok, string} | :default. This way if someone wants to customize a specific value, like dates, they can do that without reimplementing the default logic for other values.
With that, the :formatter option wouldn't have a default value, if present we call the formatter first, otherwise we don't, in both cases we follow up with the default logic if applicable.
|
Hey @kipcole9! I like the idea, just a small note on the API and we can ship it :D |
|
Ah and as for tests, please add one in kino/test/kino/data_table_test.exs Lines 249 to 261 in 93d042a The |
|
@jonatanklosko, thanks for the review and great suggestions. I've made the requested changes, tests are passing. Now I just need to add a new test as you proposed. |
|
Test case added so I think this is now ready for the next review. |
|
@kipcole9 perfect, last two nitpicks! |
|
Fixed as requested. Let me know if there's anything else? |
See the discusssion in #440.
Adds a
:formatteroption toKino.DataTable.new/2which is a 2-arity function passed thekey(column name) and cell value.The special key
:__header__is passed when formatting the column headings.