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.

Sign up for GitHub

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

Fix incorrect cell width evaluation in case of invisible symbols #39

Merged
merged 6 commits into from
Feb 7, 2020
Merged

Fix incorrect cell width evaluation in case of invisible symbols #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3cc961f
[F] Fix incorrect cell width evaluation in case of invisible symbols
seleznevae Feb 3, 2020
c596d15
[A] Add changelog entry
seleznevae Feb 4, 2020
d87f5c6
[A] Add regression test for c++ API
seleznevae Feb 5, 2020
3dff45b
[F] Fix evaluation of max buffer width for column of the table
seleznevae Feb 5, 2020
0b671b1
[F] Fix cell width evalutation
seleznevae Feb 7, 2020
caea5d4
[C] Renamed internal functions
seleznevae Feb 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- Add new table property `adding_strategy` (2 strategies available - replace(default) and insert).
- Add function `ft_row_count` (`row_count` in C++ API) to get number of rows in the table.

### Bug fixes

- Fix incorrect cell width evaluation in case of invisible symbols

### Internal

- Refactoring of code that uses vectors.
Expand Down
70 changes: 51 additions & 19 deletions lib/fort.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2091,7 +2091,10 @@ FT_INTERNAL
f_cell_t *copy_cell(f_cell_t *cell);

FT_INTERNAL
size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_geometry_type geom);
size_t cell_vis_width(const f_cell_t *cell, const f_context_t *context);

FT_INTERNAL
size_t cell_invis_codes_width(const f_cell_t *cell, const f_context_t *context);

FT_INTERNAL
size_t hint_height_cell(const f_cell_t *cell, const f_context_t *context);
Expand Down Expand Up @@ -2353,7 +2356,7 @@ enum f_cell_type get_cell_type(const f_cell_t *cell)
}

FT_INTERNAL
size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_geometry_type geom)
size_t cell_vis_width(const f_cell_t *cell, const f_context_t *context)
{
/* todo:
* At the moment min width includes paddings. Maybe it is better that min width weren't include
Expand All @@ -2374,25 +2377,35 @@ size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_
result += buffer_text_visible_width(cell->str_buffer);
}
result = MAX(result, (size_t)get_cell_property_hierarchically(properties, row, column, FT_CPROP_MIN_WIDTH));
return result;
}

if (geom == INTERN_REPR_GEOMETRY) {
char cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_cell(properties, row, column, cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(cell_style_tag);
FT_INTERNAL
size_t cell_invis_codes_width(const f_cell_t *cell, const f_context_t *context)
{
assert(cell);
assert(context);

char reset_cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_cell(properties, row, column, reset_cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_cell_style_tag);
f_table_properties_t *properties = context->table_properties;
size_t row = context->row;
size_t column = context->column;

char content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_content(properties, row, column, content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(content_style_tag);
size_t result = 0;
char cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_cell(properties, row, column, cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(cell_style_tag);

char reset_content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_content(properties, row, column, reset_content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_content_style_tag);
}
char reset_cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_cell(properties, row, column, reset_cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_cell_style_tag);

char content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_content(properties, row, column, content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(content_style_tag);

char reset_content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_content(properties, row, column, reset_content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_content_style_tag);
return result;
}

Expand Down Expand Up @@ -2424,7 +2437,7 @@ int cell_printf(f_cell_t *cell, size_t row, f_conv_context_t *cntx, size_t vis_w
const f_context_t *context = cntx->cntx;
size_t buf_len = vis_width;

if (cell == NULL || (vis_width < hint_width_cell(cell, context, VISIBLE_GEOMETRY))) {
if (cell == NULL || (vis_width < cell_vis_width(cell, context))) {
return -1;
}

Expand Down Expand Up @@ -7001,6 +7014,7 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
return FT_ERROR;
}

size_t max_invis_codepoints = 0;
size_t cols = 0;
size_t rows = 0;
int status = get_table_sizes(table, &rows, &cols);
Expand Down Expand Up @@ -7030,7 +7044,7 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
if (cell) {
switch (get_cell_type(cell)) {
case COMMON_CELL:
col_width_arr[col] = MAX(col_width_arr[col], hint_width_cell(cell, &context, geom));
col_width_arr[col] = MAX(col_width_arr[col], cell_vis_width(cell, &context));
break;
case GROUP_MASTER_CELL:
combined_cells_found = 1;
Expand All @@ -7049,6 +7063,21 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
}
}
}

if (geom == INTERN_REPR_GEOMETRY) {
max_invis_codepoints = 0;
for (row = 0; row < rows; ++row) {
const f_row_t *row_p = get_row_c(table, row);
const f_cell_t *cell = get_cell_c(row_p, col);
if (!cell)
continue;
context.column = col;
context.row = row;
size_t inv_codepoints = cell_invis_codes_width(cell, &context);
max_invis_codepoints = MAX(max_invis_codepoints, inv_codepoints);
}
col_width_arr[col] += max_invis_codepoints;
}
}

if (combined_cells_found) {
Expand All @@ -7061,7 +7090,10 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
context.row = row;
if (cell) {
if (get_cell_type(cell) == GROUP_MASTER_CELL) {
size_t hint_width = hint_width_cell(cell, &context, geom);
size_t hint_width = cell_vis_width(cell, &context);
if (geom == INTERN_REPR_GEOMETRY) {
hint_width += cell_invis_codes_width(cell, &context);
}
size_t slave_col = col + group_cell_number(row_p, col);
size_t cur_adj_col = col;
size_t group_width = col_width_arr[col];
Expand Down
42 changes: 26 additions & 16 deletions src/cell.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ enum f_cell_type get_cell_type(const f_cell_t *cell)
}

FT_INTERNAL
size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_geometry_type geom)
size_t cell_vis_width(const f_cell_t *cell, const f_context_t *context)
{
/* todo:
* At the moment min width includes paddings. Maybe it is better that min width weren't include
Expand All @@ -86,25 +86,35 @@ size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_
result += buffer_text_visible_width(cell->str_buffer);
}
result = MAX(result, (size_t)get_cell_property_hierarchically(properties, row, column, FT_CPROP_MIN_WIDTH));
return result;
}

if (geom == INTERN_REPR_GEOMETRY) {
char cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_cell(properties, row, column, cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(cell_style_tag);
FT_INTERNAL
size_t cell_invis_codes_width(const f_cell_t *cell, const f_context_t *context)
{
assert(cell);
assert(context);

char reset_cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_cell(properties, row, column, reset_cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_cell_style_tag);
f_table_properties_t *properties = context->table_properties;
size_t row = context->row;
size_t column = context->column;

char content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_content(properties, row, column, content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(content_style_tag);
size_t result = 0;
char cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_cell(properties, row, column, cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(cell_style_tag);

char reset_content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_content(properties, row, column, reset_content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_content_style_tag);
}
char reset_cell_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_cell(properties, row, column, reset_cell_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_cell_style_tag);

char content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_style_tag_for_content(properties, row, column, content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(content_style_tag);

char reset_content_style_tag[TEXT_STYLE_TAG_MAX_SIZE];
get_reset_style_tag_for_content(properties, row, column, reset_content_style_tag, TEXT_STYLE_TAG_MAX_SIZE);
result += strlen(reset_content_style_tag);
return result;
}

Expand Down Expand Up @@ -136,7 +146,7 @@ int cell_printf(f_cell_t *cell, size_t row, f_conv_context_t *cntx, size_t vis_w
const f_context_t *context = cntx->cntx;
size_t buf_len = vis_width;

if (cell == NULL || (vis_width < hint_width_cell(cell, context, VISIBLE_GEOMETRY))) {
if (cell == NULL || (vis_width < cell_vis_width(cell, context))) {
return -1;
}

Expand Down
5 changes: 4 additions & 1 deletion src/cell.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ FT_INTERNAL
f_cell_t *copy_cell(f_cell_t *cell);

FT_INTERNAL
size_t hint_width_cell(const f_cell_t *cell, const f_context_t *context, enum f_geometry_type geom);
size_t cell_vis_width(const f_cell_t *cell, const f_context_t *context);

FT_INTERNAL
size_t cell_invis_codes_width(const f_cell_t *cell, const f_context_t *context);

FT_INTERNAL
size_t hint_height_cell(const f_cell_t *cell, const f_context_t *context);
Expand Down
23 changes: 21 additions & 2 deletions src/table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
return FT_ERROR;
}

size_t max_invis_codepoints = 0;
size_t cols = 0;
size_t rows = 0;
int status = get_table_sizes(table, &rows, &cols);
Expand Down Expand Up @@ -172,7 +173,7 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
if (cell) {
switch (get_cell_type(cell)) {
case COMMON_CELL:
col_width_arr[col] = MAX(col_width_arr[col], hint_width_cell(cell, &context, geom));
col_width_arr[col] = MAX(col_width_arr[col], cell_vis_width(cell, &context));
break;
case GROUP_MASTER_CELL:
combined_cells_found = 1;
Expand All @@ -191,6 +192,21 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
}
}
}

if (geom == INTERN_REPR_GEOMETRY) {
max_invis_codepoints = 0;
for (row = 0; row < rows; ++row) {
const f_row_t *row_p = get_row_c(table, row);
const f_cell_t *cell = get_cell_c(row_p, col);
if (!cell)
continue;
context.column = col;
context.row = row;
size_t inv_codepoints = cell_invis_codes_width(cell, &context);
max_invis_codepoints = MAX(max_invis_codepoints, inv_codepoints);
}
col_width_arr[col] += max_invis_codepoints;
}
}

if (combined_cells_found) {
Expand All @@ -203,7 +219,10 @@ f_status table_rows_and_cols_geometry(const ft_table_t *table,
context.row = row;
if (cell) {
if (get_cell_type(cell) == GROUP_MASTER_CELL) {
size_t hint_width = hint_width_cell(cell, &context, geom);
size_t hint_width = cell_vis_width(cell, &context);
if (geom == INTERN_REPR_GEOMETRY) {
hint_width += cell_invis_codes_width(cell, &context);
}
size_t slave_col = col + group_cell_number(row_p, col);
size_t cur_adj_col = col;
size_t group_width = col_width_arr[col];
Expand Down
108 changes: 108 additions & 0 deletions tests/bb_tests/test_table_basic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,114 @@ void test_bug_fixes(void)
ft_destroy_table(table);
}
#endif /* FT_HAVE_UTF8 */

SCENARIO("Issue 37 - https://github.com/seleznevae/libfort/issues/37") {
ft_table_t *table = ft_create_table();
ft_set_border_style(table, FT_BASIC_STYLE);
ft_set_cell_prop(table, 0, FT_ANY_COLUMN, FT_CPROP_ROW_TYPE, FT_ROW_HEADER);
ft_write_ln(table, "xxx");
ft_write_ln(table,
"1\n2\n3\n4\n5\n6\n7\n8\n9\n0\n1\n2\n3\n4\n5\n6\n7\n8");
ft_set_cell_prop(table, 1, 0, FT_CPROP_CONT_FG_COLOR, FT_COLOR_RED);

const char *table_str = ft_to_string(table);
assert_true(table_str != NULL);
const char *table_str_etalon =
"+-----+\n"
"| xxx |\n"
"+-----+\n"
"| \033[31m1\033[0m |\n"
"| \033[31m2\033[0m |\n"
"| \033[31m3\033[0m |\n"
"| \033[31m4\033[0m |\n"
"| \033[31m5\033[0m |\n"
"| \033[31m6\033[0m |\n"
"| \033[31m7\033[0m |\n"
"| \033[31m8\033[0m |\n"
"| \033[31m9\033[0m |\n"
"| \033[31m0\033[0m |\n"
"| \033[31m1\033[0m |\n"
"| \033[31m2\033[0m |\n"
"| \033[31m3\033[0m |\n"
"| \033[31m4\033[0m |\n"
"| \033[31m5\033[0m |\n"
"| \033[31m6\033[0m |\n"
"| \033[31m7\033[0m |\n"
"| \033[31m8\033[0m |\n"
"+-----+\n";
assert_str_equal(table_str, table_str_etalon);

ft_destroy_table(table);
}

SCENARIO("Issue 37 - https://github.com/seleznevae/libfort/issues/37") {
ft_table_t *table = ft_create_table();
ft_set_border_style(table, FT_BASIC_STYLE);
ft_set_cell_prop(table, 0, FT_ANY_COLUMN, FT_CPROP_ROW_TYPE, FT_ROW_HEADER);

ft_write_ln(table, "hdr1", "hdr2", "xxx");
ft_write_ln(table, "3", "",
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||||||||||||||||||||||||||\n"
"||||||");

ft_set_cell_prop(table, 1, FT_ANY_COLUMN, FT_CPROP_CONT_FG_COLOR, FT_COLOR_RED);
const char *table_str = ft_to_string(table);
assert_true(table_str != NULL);

const char *table_str_etalon =
"+------+------+--------------------------------+\n"
"| hdr1 | hdr2 | xxx |\n"
"+------+------+--------------------------------+\n"
"| \033[31m3\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||||||||||||||||||||||||||\033[0m |\n"
"|\033[31m\033[0m |\033[31m\033[0m | \033[31m||||||\033[0m |\n"
"+------+------+--------------------------------+\n";
assert_str_equal(table_str, table_str_etalon);
ft_destroy_table(table);
}
}

void test_table_basic(void)
Expand Down
Loading