From a28ed79993e6fb8542d917e9270b0245860f35cb Mon Sep 17 00:00:00 2001 From: Martin Mares Date: Fri, 25 Jul 2014 14:01:52 +0200 Subject: [PATCH] Table printer: Review comments --- ucw/table.c | 8 ++++++-- ucw/table.h | 20 +++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/ucw/table.c b/ucw/table.c index 8b2660cd..bb64dcff 100644 --- a/ucw/table.c +++ b/ucw/table.c @@ -64,6 +64,7 @@ static struct table *table_make_instance(const struct table_template *tbl_templa new_inst->col_delimiter = tbl_template->col_delimiter; new_inst->print_header = true; new_inst->out = 0; + // FIXME: last_printed_col is not used anywhere new_inst->last_printed_col = -1; new_inst->row_printing_started = false; new_inst->col_out = -1; @@ -75,11 +76,13 @@ static struct table *table_make_instance(const struct table_template *tbl_templa struct table *table_init(const struct table_template *tbl_template) { struct table *tbl = table_make_instance(tbl_template); + // FIXME: What remains of table_init()? Just combine it with table_make_instance() if(!tbl->formatter) { tbl->formatter = &table_fmt_human_readable; } + // FIXME: This is already set by table_make_instance() tbl->print_header = true; // by default, print header return tbl; } @@ -92,7 +95,7 @@ void table_cleanup(struct table *tbl) // TODO: test default column order static void table_make_default_column_order(struct table *tbl) { - int *col_order_int = mp_alloc_zero(tbl->pool, sizeof(int) * tbl->column_count); // FIXME: use stack instead of memory pool + int *col_order_int = mp_alloc_zero(tbl->pool, sizeof(int) * tbl->column_count); // FIXME: use stack instead of memory pool (yes, please!) for(int i = 0; i < tbl->column_count; i++) { col_order_int[i] = i; } @@ -116,7 +119,7 @@ void table_start(struct table *tbl, struct fastbuf *out) mp_save(tbl->pool, &tbl->pool_state); - ASSERT_MSG(tbl->col_delimiter, "In-between column delimiter not specified."); + ASSERT_MSG(tbl->col_delimiter, "Column delimiter not specified."); } void table_end(struct table *tbl) @@ -296,6 +299,7 @@ const char * table_set_col_order_by_name(struct table *tbl, const char *col_orde /*** Table cells ***/ +// FIXME: This should be table_col_raw(), shouldn't it? static void table_set_raw(struct table *tbl, int col_templ, const char *col_content) { TBL_COL_ITER_START(tbl, col_templ, curr_col_ptr, curr_col) { diff --git a/ucw/table.h b/ucw/table.h index 783c802e..a7c0236e 100644 --- a/ucw/table.h +++ b/ucw/table.h @@ -83,6 +83,7 @@ struct table_column { const char * (*set_col_opt)(struct table *tbl, uint col, const char *value); // [*] process table option for a column instance + // FIXME: Comment on arguments and return value }; // FIXME: is it correct to have idx and col_def? idx is sufficient and in fact a duplicity of idx @@ -317,12 +318,17 @@ int table_get_col_idx(struct table *tbl, const char *col_name); /** - * Sets a string option to an instance of a columnt type. This is the default version that checks + * Sets a string option to an instance of a column type. This is the default version that checks * whether the xtype::parse_fmt can be called and calls it. However, there are situation in which * the xtype::parse_fmt is not sufficient, e.g., column decoration, post-processing, etc. * * Each struct table_column has a pointer to a customized version of table_set_col_opt which is * called instead of this (default) version of table_set_col_opt + * + * FIXME: Make table_set_col_opt() a front-end function used by everybody, + * which checks if the set_col_opt hook is defined and either calls it or + * processes the options in the generic way. Nobody else should call the + * hook directly. **/ const char *table_set_col_opt(struct table *tbl, uint col_idx, const char *col_opt); @@ -333,15 +339,17 @@ const char *table_set_col_opt(struct table *tbl, uint col_idx, const char *col_o const char *table_get_col_list(struct table *tbl); /** - * Sets the order in which the columns are printed. The @col_order parameter is used until - * @table_end() or @table_cleanup() is called. The table converts the integers in @col_order into - * internal representation stored in @column_order. Options to column instances can be set using - * @table_set_col_opt. + * Sets the order in which the columns are printed. + * The table converts the integers in @col_order into an internal representation stored + * in `column_order`. Options to column instances can be set using @table_set_col_opt(). **/ void table_set_col_order(struct table *tbl, int *col_order, int col_order_size); /** * Returns true if col_idx will be printed, false otherwise. + * + * FIXME: Naming of arguments is confusing. @col_idx sometimes indexes + * columns, but sometimes their instances. **/ bool table_col_is_printed(struct table *tbl, uint col_idx); @@ -381,6 +389,8 @@ void table_set_formatter(struct table *tbl, const struct table_formatter *fmt); * | `fmt` | `human`/`machine`/`block` | set table formatter to one of the built-in formatters * | `col-delim`| string | set column delimiter * | `cells` | string | set column format mode + * | `raw` | 'none' | set column format to raw data + * | `pretty` | 'none' | set column format to pretty-printing * |=================================================================================================== **/ const char *table_set_option_value(struct table *tbl, const char *key, const char *value); -- 2.39.2