tableprinter: code cleanup

 - removed last_printed_col
 - update of initialization of column orders in table_set_col_order
 - update of table_set_col_opt: called die if table_column::set_col_opt
   is a pointer to table_set_col_opt.
 - update of documentation of table_set_col_opt

diff --git a/ucw/table.c b/ucw/table.c
index bb64dcff155410a243ae5d0b9699d9cc3e3f10ca..a13f6e6a667fc2cbce435cc87cc475a86b43909b 100644 (file)
--- a/ucw/table.c
+++ b/ucw/table.c
@@ -64,12 +64,13 @@ 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;
   new_inst->formatter = tbl_template->formatter;
-  new_inst->data = NULL;
+  if(!new_inst->formatter) {
+    new_inst->formatter = &table_fmt_human_readable;
+  }
+  new_inst->formatter_data = NULL;
   return new_inst;
 }
 
@@ -78,12 +79,7 @@ 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;
 }
 
@@ -95,7 +91,8 @@ 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 (yes, please!)
+  int *col_order_int = alloca(sizeof(int) * tbl->column_count);
+
   for(int i = 0; i < tbl->column_count; i++) {
     col_order_int[i] = i;
   }
@@ -104,7 +101,6 @@ static void table_make_default_column_order(struct table *tbl)
 
 void table_start(struct table *tbl, struct fastbuf *out)
 {
-  tbl->last_printed_col = -1;
   tbl->row_printing_started = false;
   tbl->out = out;
 
@@ -124,7 +120,6 @@ void table_start(struct table *tbl, struct fastbuf *out)
 
 void table_end(struct table *tbl)
 {
-  tbl->last_printed_col = -1;
   tbl->row_printing_started = false;
 
   mp_restore(tbl->pool, &tbl->pool_state);
@@ -193,7 +188,8 @@ void table_set_col_order(struct table *tbl, int *col_order, int cols_to_output)
     int col_idx = col_order[i];
     tbl->column_order[i].idx = col_idx;
     tbl->column_order[i].col_def = tbl->columns + col_idx;
-    tbl->column_order[i].cell_content = NULL;
+    tbl->column_order[i].cell_content = NULL; // it is in fact initialized by mp_alloc_zero, but for completeness ...
+    tbl->column_order[i].next_column = -1;
     tbl->column_order[i].fmt = tbl->columns[col_idx].fmt;
   }
   table_update_ll(tbl);
@@ -218,22 +214,28 @@ static char * table_parse_col_arg(char *col_def)
   return left_br;
 }
 
-const char *table_set_col_opt(struct table *tbl, uint col_idx, const char *col_opt)
+const char *table_set_col_opt(struct table *tbl, uint col_inst_idx, const char *col_opt)
 {
-  const struct table_column *col_def = tbl->column_order[col_idx].col_def;
-  if(col_def && col_def->set_col_opt && col_def->set_col_opt != table_set_col_opt) {
-    return col_def->set_col_opt(tbl, col_idx, col_opt);
+  const struct table_column *col_def = tbl->column_order[col_inst_idx].col_def;
+
+  // Make sure that we do not call table_set_col_opt, which would
+  // result in an infinite recursion.
+  if(col_def && col_def->set_col_opt) {
+    if(col_def->set_col_opt == table_set_col_opt) {
+      die("table_set_col_opt should not be used as a struct table_column::set_col_opt hook");
+    }
+    return col_def->set_col_opt(tbl, col_inst_idx, col_opt);
   }
 
   if(col_def && col_def->type_def && col_def->type_def->parse_fmt) {
     uint fmt = 0;
     const char *tmp_err = col_def->type_def->parse_fmt(col_opt, &fmt, tbl->pool);
     if(tmp_err) return tmp_err;
-    tbl->column_order[col_idx].fmt = fmt;
+    tbl->column_order[col_inst_idx].fmt = fmt;
     return NULL;
   }
 
-  return mp_printf(tbl->pool, "Invalid column format option: '%s' for column %d.", col_opt, col_idx);
+  return mp_printf(tbl->pool, "Invalid column format option: '%s' for column %d.", col_opt, col_inst_idx);
 }
 
 /**
@@ -299,8 +301,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)
+static void table_col_raw(struct table *tbl, int col_templ, const char *col_content)
 {
   TBL_COL_ITER_START(tbl, col_templ, curr_col_ptr, curr_col) {
     curr_col_ptr->cell_content = col_content;
@@ -311,7 +312,6 @@ void table_col_generic_format(struct table *tbl, int col, void *value, const str
 {
   ASSERT_MSG(col < tbl->column_count && col >= 0, "Table column %d does not exist.", col);
   ASSERT(tbl->columns[col].type_def == COL_TYPE_ANY || expected_type == tbl->columns[col].type_def);
-  tbl->last_printed_col = col;
   tbl->row_printing_started = true;
   TBL_COL_ITER_START(tbl, col, curr_col, curr_col_idx) {
     enum xtype_fmt fmt = curr_col->fmt;
@@ -322,12 +322,11 @@ void table_col_generic_format(struct table *tbl, int col, void *value, const str
 void table_col_printf(struct table *tbl, int col, const char *fmt, ...)
 {
   ASSERT_MSG(col < tbl->column_count && col >= 0, "Table column %d does not exist.", col);
-  tbl->last_printed_col = col;
   tbl->row_printing_started = true;
   va_list args;
   va_start(args, fmt);
   char *cell_content = mp_vprintf(tbl->pool, fmt, args);
-  table_set_raw(tbl, col, cell_content);
+  table_col_raw(tbl, col, cell_content);
   va_end(args);
 }
 
@@ -347,7 +346,6 @@ void table_reset_row(struct table *tbl)
     tbl->column_order[i].cell_content = NULL;
   }
   mp_restore(tbl->pool, &tbl->pool_state);
-  tbl->last_printed_col = -1;
   tbl->row_printing_started = false;
 }
 
@@ -372,7 +370,7 @@ struct fastbuf *table_col_fbstart(struct table *tbl, int col)
 void table_col_fbend(struct table *tbl)
 {
   char *cell_content = fbpool_end(&tbl->fb_col_out);
-  table_set_raw(tbl, tbl->col_out, cell_content);
+  table_col_raw(tbl, tbl->col_out, cell_content);
   tbl->col_out = -1;
 }
 

diff --git a/ucw/table.h b/ucw/table.h
index 9b1d2a1a44cf1bd5d92f7033cc44089b4b5bafff..017e0652f01802871865f203bb91e815e827973d 100644 (file)
--- a/ucw/table.h
+++ b/ucw/table.h
@@ -81,8 +81,8 @@ struct table_column {
   uint fmt;                     // [*] default format of the column
   const struct xtype *type_def; // [*] pointer to xtype of this column
 
-  const char * (*set_col_opt)(struct table *tbl, uint col, const char *value);
-       // [*] process table option for a column instance
+  const char * (*set_col_opt)(struct table *tbl, uint col_inst_idx, const char *col_opt);
+       // [*] process table option for a column instance. @col_inst_idx is the index of the column to which the @col_opt is set.
        // FIXME: Comment on arguments and return value
 };
 
@@ -129,9 +129,7 @@ struct table {
   bool print_header;                   // [*] false indicates that table header should not be printed
 
   struct fastbuf *out;                 // [*] Fastbuffer to which the table is printed
-  int last_printed_col;                        // Index of the last column which was set. -1 indicates start of row.
-                                       // Used for example for appending to the current column.
-  bool row_printing_started;           // Indicates that a row has been started. Duplicates last_printed_col, but harmlessly.
+  bool row_printing_started;           // Indicates that a row has been started.
   struct fbpool fb_col_out;            // Per-cell fastbuf, see table_col_fbstart()
   int col_out;                         // Index of the column that is currently printed using fb_col_out
 
@@ -329,6 +327,8 @@ int table_get_col_idx(struct table *tbl, const char *col_name);
  * 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.
+ *     RK: that is the current solution the only confusion can be that
+ *     the hook and this function has the same prototype.
  **/
 const char *table_set_col_opt(struct table *tbl, uint col_idx, const char *col_opt);
 
@@ -342,6 +342,9 @@ const char *table_get_col_list(struct table *tbl);
  * 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().
+ *
+ * FIXME: add a function to the interface that accepts a pointer to an
+ * array of table_col_instance.
  **/
 void table_set_col_order(struct table *tbl, int *col_order, int col_order_size);