Message ID | 1481126154-6515-1-git-send-email-aconole@redhat.com |
---|---|
State | Changes Requested |
Headers | show |
On Wed, Dec 07, 2016 at 10:55:54AM -0500, Aaron Conole wrote: > Adds a new --format="xx={0},..." to the table library, which allows > users to make table output formats to match their own needs. > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > v1 url: > https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325235.html > > TODO: Still needs unit tests to check the format strings > One thing I want to do is put in the idea of string length to the > grammar. Probably will be a few other additions (ex: text which is > omitted when the corresponding column value is empty). > > NOTE: Submitted for early feedback Can you give an example of a use case? If we're going to add this feature for formatting, I'd like to know that it's going to be genuinely useful to users.
Ben Pfaff <blp@ovn.org> writes: > On Wed, Dec 07, 2016 at 10:55:54AM -0500, Aaron Conole wrote: >> Adds a new --format="xx={0},..." to the table library, which allows >> users to make table output formats to match their own needs. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> v1 url: >> https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325235.html >> >> TODO: Still needs unit tests to check the format strings >> One thing I want to do is put in the idea of string length to the >> grammar. Probably will be a few other additions (ex: text which is >> omitted when the corresponding column value is empty). >> >> NOTE: Submitted for early feedback > > Can you give an example of a use case? If we're going to add this > feature for formatting, I'd like to know that it's going to be genuinely > useful to users. Yes, sorry - I should have re-included my previous cover letter; I am currently asked by some openstack folks to aid with 'debugability' - specifically, they want a change to the openflow printing such that they have data like: COL1 COL2 COL3 ==== ==== ==== They want to choose the columns 'as-needed', and I think they also want to change the formats. Rather than code up multiple custom styles, I'd like to just give them the ability to change output formats as they wish. To accomplish this, I'm going down the following route: 1. Add formatting option to the lib/table.{c,h}, such that we can still express the existing output. 2. Change ofp-print to use a table for flow dumping, with a default format that preserves the current format, and the option of changing the format. Hopefully, I've explained it. Thanks, -Aaron
On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Wed, Dec 07, 2016 at 10:55:54AM -0500, Aaron Conole wrote: > >> Adds a new --format="xx={0},..." to the table library, which allows > >> users to make table output formats to match their own needs. > >> > >> Signed-off-by: Aaron Conole <aconole@redhat.com> > >> --- > >> v1 url: > >> https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325235.html > >> > >> TODO: Still needs unit tests to check the format strings > >> One thing I want to do is put in the idea of string length to the > >> grammar. Probably will be a few other additions (ex: text which is > >> omitted when the corresponding column value is empty). > >> > >> NOTE: Submitted for early feedback > > > > Can you give an example of a use case? If we're going to add this > > feature for formatting, I'd like to know that it's going to be genuinely > > useful to users. > > Yes, sorry - I should have re-included my previous cover letter; > > I am currently asked by some openstack folks to aid with 'debugability' > - specifically, they want a change to the openflow printing such that > they have data like: > > COL1 COL2 COL3 > ==== ==== ==== > > They want to choose the columns 'as-needed', and I think they also want > to change the formats. Rather than code up multiple custom styles, I'd > like to just give them the ability to change output formats as they > wish. I guess it just sounds very similar to --format=table. > To accomplish this, I'm going down the following route: > > 1. Add formatting option to the lib/table.{c,h}, such that we can > still express the existing output. > > 2. Change ofp-print to use a table for flow dumping, with a default > format that preserves the current format, and the option of changing > the format. #2 is interesting.
Ben Pfaff <blp@ovn.org> writes: > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: >> Ben Pfaff <blp@ovn.org> writes: >> >> > On Wed, Dec 07, 2016 at 10:55:54AM -0500, Aaron Conole wrote: >> >> Adds a new --format="xx={0},..." to the table library, which allows >> >> users to make table output formats to match their own needs. >> >> >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> >> --- >> >> v1 url: >> >> https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325235.html >> >> >> >> TODO: Still needs unit tests to check the format strings >> >> One thing I want to do is put in the idea of string length to the >> >> grammar. Probably will be a few other additions (ex: text which is >> >> omitted when the corresponding column value is empty). >> >> >> >> NOTE: Submitted for early feedback >> > >> > Can you give an example of a use case? If we're going to add this >> > feature for formatting, I'd like to know that it's going to be genuinely >> > useful to users. >> >> Yes, sorry - I should have re-included my previous cover letter; >> >> I am currently asked by some openstack folks to aid with 'debugability' >> - specifically, they want a change to the openflow printing such that >> they have data like: >> >> COL1 COL2 COL3 >> ==== ==== ==== >> >> They want to choose the columns 'as-needed', and I think they also want >> to change the formats. Rather than code up multiple custom styles, I'd >> like to just give them the ability to change output formats as they >> wish. > > I guess it just sounds very similar to --format=table. Agreed, this specific format is. >> To accomplish this, I'm going down the following route: >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can >> still express the existing output. >> >> 2. Change ofp-print to use a table for flow dumping, with a default >> format that preserves the current format, and the option of changing >> the format. > > #2 is interesting. I don't currently see how to implement #2, while preserving the existing output, unless I implement #1. Is there a way to accomplish this that I'm missing? If not, is #2 compelling enough to accept #1? Thanks for your time and for looking at this, Ben.
On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: > >> Ben Pfaff <blp@ovn.org> writes: > >> To accomplish this, I'm going down the following route: > >> > >> 1. Add formatting option to the lib/table.{c,h}, such that we can > >> still express the existing output. > >> > >> 2. Change ofp-print to use a table for flow dumping, with a default > >> format that preserves the current format, and the option of changing > >> the format. > > > > #2 is interesting. > > I don't currently see how to implement #2, while preserving the existing > output, unless I implement #1. Is there a way to accomplish this that > I'm missing? If not, is #2 compelling enough to accept #1? I don't understand how you are going to preserve the default format even with #1. I assumed you were going to need to write a separate code path to do that. Can you explain your plan?
Ben Pfaff <blp@ovn.org> writes: > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: >> Ben Pfaff <blp@ovn.org> writes: >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: >> >> Ben Pfaff <blp@ovn.org> writes: >> >> To accomplish this, I'm going down the following route: >> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can >> >> still express the existing output. >> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default >> >> format that preserves the current format, and the option of changing >> >> the format. >> > >> > #2 is interesting. >> >> I don't currently see how to implement #2, while preserving the existing >> output, unless I implement #1. Is there a way to accomplish this that >> I'm missing? If not, is #2 compelling enough to accept #1? > > I don't understand how you are going to preserve the default format even > with #1. I assumed you were going to need to write a separate code path > to do that. No - that's my last resort. > Can you explain your plan? I was thinking of creating a table which had column headings that were, example: actions, in_port, priority, table number, etc. Then a format string such as: [priority={priority},][table={tableno},]... where things in the [] would only be printed if all elements in the row were filled in, could preserve the original output (without the full working code set it might be difficult to see what I mean), while also offering the existing --table=XXX output. In that sense, we could switch from dynamic string in all of the ofp-print (and sub-calls) to just filling in column data - then have pre-set table format strings that can be switched between. That also means any future manupulation (ex: changing the colors, or something else) could just be handled the same way existing prints are handled. -Aaron
On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: > >> Ben Pfaff <blp@ovn.org> writes: > >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: > >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> To accomplish this, I'm going down the following route: > >> >> > >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can > >> >> still express the existing output. > >> >> > >> >> 2. Change ofp-print to use a table for flow dumping, with a default > >> >> format that preserves the current format, and the option of changing > >> >> the format. > >> > > >> > #2 is interesting. > >> > >> I don't currently see how to implement #2, while preserving the existing > >> output, unless I implement #1. Is there a way to accomplish this that > >> I'm missing? If not, is #2 compelling enough to accept #1? > > > > I don't understand how you are going to preserve the default format even > > with #1. I assumed you were going to need to write a separate code path > > to do that. > > No - that's my last resort. > > > Can you explain your plan? > > I was thinking of creating a table which had column headings that were, > example: > > actions, in_port, priority, table number, etc. > > Then a format string such as: > > [priority={priority},][table={tableno},]... > > where things in the [] would only be printed if all elements in the row > were filled in, could preserve the original output (without the full > working code set it might be difficult to see what I mean), while also > offering the existing --table=XXX output. This may be challenging because of the number of columns. in_port is easy enough, but OVS has dozens (at least) of possible fields. It also has a nonuniform way to display fields. For example, it shows the Ethernet type in a general form as "dl_type=0x1234", but if it's an IP packet then it just says "ip" instead, and if it's TCP then it just says "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on.
Ben Pfaff <blp@ovn.org> writes: > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote: >> Ben Pfaff <blp@ovn.org> writes: >> >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: >> >> Ben Pfaff <blp@ovn.org> writes: >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: >> >> >> Ben Pfaff <blp@ovn.org> writes: >> >> >> To accomplish this, I'm going down the following route: >> >> >> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can >> >> >> still express the existing output. >> >> >> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default >> >> >> format that preserves the current format, and the option of changing >> >> >> the format. >> >> > >> >> > #2 is interesting. >> >> >> >> I don't currently see how to implement #2, while preserving the existing >> >> output, unless I implement #1. Is there a way to accomplish this that >> >> I'm missing? If not, is #2 compelling enough to accept #1? >> > >> > I don't understand how you are going to preserve the default format even >> > with #1. I assumed you were going to need to write a separate code path >> > to do that. >> >> No - that's my last resort. >> >> > Can you explain your plan? >> >> I was thinking of creating a table which had column headings that were, >> example: >> >> actions, in_port, priority, table number, etc. >> >> Then a format string such as: >> >> [priority={priority},][table={tableno},]... >> >> where things in the [] would only be printed if all elements in the row >> were filled in, could preserve the original output (without the full >> working code set it might be difficult to see what I mean), while also >> offering the existing --table=XXX output. > > This may be challenging because of the number of columns. in_port is > easy enough, but OVS has dozens (at least) of possible fields. It also > has a nonuniform way to display fields. For example, it shows the > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP > packet then it just says "ip" instead, and if it's TCP then it just says > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on. I enjoy a challenge from time to time ;-) Moreover, I'm willing to do the work. Agreed, there's lots of it (dozens is right), but I think it's an elegant solution.
On Tue, Dec 13, 2016 at 04:43:32PM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote: > >> Ben Pfaff <blp@ovn.org> writes: > >> > >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: > >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: > >> >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> >> To accomplish this, I'm going down the following route: > >> >> >> > >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can > >> >> >> still express the existing output. > >> >> >> > >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default > >> >> >> format that preserves the current format, and the option of changing > >> >> >> the format. > >> >> > > >> >> > #2 is interesting. > >> >> > >> >> I don't currently see how to implement #2, while preserving the existing > >> >> output, unless I implement #1. Is there a way to accomplish this that > >> >> I'm missing? If not, is #2 compelling enough to accept #1? > >> > > >> > I don't understand how you are going to preserve the default format even > >> > with #1. I assumed you were going to need to write a separate code path > >> > to do that. > >> > >> No - that's my last resort. > >> > >> > Can you explain your plan? > >> > >> I was thinking of creating a table which had column headings that were, > >> example: > >> > >> actions, in_port, priority, table number, etc. > >> > >> Then a format string such as: > >> > >> [priority={priority},][table={tableno},]... > >> > >> where things in the [] would only be printed if all elements in the row > >> were filled in, could preserve the original output (without the full > >> working code set it might be difficult to see what I mean), while also > >> offering the existing --table=XXX output. > > > > This may be challenging because of the number of columns. in_port is > > easy enough, but OVS has dozens (at least) of possible fields. It also > > has a nonuniform way to display fields. For example, it shows the > > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP > > packet then it just says "ip" instead, and if it's TCP then it just says > > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on. > > I enjoy a challenge from time to time ;-) > > Moreover, I'm willing to do the work. Agreed, there's lots of it > (dozens is right), but I think it's an elegant solution. Do you have a basic plan for how to do it? There's definitely room for better display here, but I want the code to be clean and maintainable.
Ben Pfaff <blp@ovn.org> writes: > On Tue, Dec 13, 2016 at 04:43:32PM -0500, Aaron Conole wrote: >> Ben Pfaff <blp@ovn.org> writes: >> >> > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote: >> >> Ben Pfaff <blp@ovn.org> writes: >> >> >> >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: >> >> >> Ben Pfaff <blp@ovn.org> writes: >> >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: >> >> >> >> Ben Pfaff <blp@ovn.org> writes: >> >> >> >> To accomplish this, I'm going down the following route: >> >> >> >> >> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can >> >> >> >> still express the existing output. >> >> >> >> >> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default >> >> >> >> format that preserves the current format, and the option of changing >> >> >> >> the format. >> >> >> > >> >> >> > #2 is interesting. >> >> >> >> >> >> I don't currently see how to implement #2, while preserving the existing >> >> >> output, unless I implement #1. Is there a way to accomplish this that >> >> >> I'm missing? If not, is #2 compelling enough to accept #1? >> >> > >> >> > I don't understand how you are going to preserve the default format even >> >> > with #1. I assumed you were going to need to write a separate code path >> >> > to do that. >> >> >> >> No - that's my last resort. >> >> >> >> > Can you explain your plan? >> >> >> >> I was thinking of creating a table which had column headings that were, >> >> example: >> >> >> >> actions, in_port, priority, table number, etc. >> >> >> >> Then a format string such as: >> >> >> >> [priority={priority},][table={tableno},]... >> >> >> >> where things in the [] would only be printed if all elements in the row >> >> were filled in, could preserve the original output (without the full >> >> working code set it might be difficult to see what I mean), while also >> >> offering the existing --table=XXX output. >> > >> > This may be challenging because of the number of columns. in_port is >> > easy enough, but OVS has dozens (at least) of possible fields. It also >> > has a nonuniform way to display fields. For example, it shows the >> > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP >> > packet then it just says "ip" instead, and if it's TCP then it just says >> > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on. >> >> I enjoy a challenge from time to time ;-) >> >> Moreover, I'm willing to do the work. Agreed, there's lots of it >> (dozens is right), but I think it's an elegant solution. > > Do you have a basic plan for how to do it? There's definitely room for > better display here, but I want the code to be clean and maintainable. So, every plan that I've gone through starts with a separate code path for the table approach if I want to stage it. It probably makes sense to start there, and add the fields I've been requested to add, first, and as part of the same series hook up ovs-ofctl dump-flows to call that new code path. Does that make sense?
On Wed, Dec 21, 2016 at 10:07:51AM -0500, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Tue, Dec 13, 2016 at 04:43:32PM -0500, Aaron Conole wrote: > >> Ben Pfaff <blp@ovn.org> writes: > >> > >> > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote: > >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> > >> >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote: > >> >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote: > >> >> >> >> Ben Pfaff <blp@ovn.org> writes: > >> >> >> >> To accomplish this, I'm going down the following route: > >> >> >> >> > >> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can > >> >> >> >> still express the existing output. > >> >> >> >> > >> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default > >> >> >> >> format that preserves the current format, and the option of changing > >> >> >> >> the format. > >> >> >> > > >> >> >> > #2 is interesting. > >> >> >> > >> >> >> I don't currently see how to implement #2, while preserving the existing > >> >> >> output, unless I implement #1. Is there a way to accomplish this that > >> >> >> I'm missing? If not, is #2 compelling enough to accept #1? > >> >> > > >> >> > I don't understand how you are going to preserve the default format even > >> >> > with #1. I assumed you were going to need to write a separate code path > >> >> > to do that. > >> >> > >> >> No - that's my last resort. > >> >> > >> >> > Can you explain your plan? > >> >> > >> >> I was thinking of creating a table which had column headings that were, > >> >> example: > >> >> > >> >> actions, in_port, priority, table number, etc. > >> >> > >> >> Then a format string such as: > >> >> > >> >> [priority={priority},][table={tableno},]... > >> >> > >> >> where things in the [] would only be printed if all elements in the row > >> >> were filled in, could preserve the original output (without the full > >> >> working code set it might be difficult to see what I mean), while also > >> >> offering the existing --table=XXX output. > >> > > >> > This may be challenging because of the number of columns. in_port is > >> > easy enough, but OVS has dozens (at least) of possible fields. It also > >> > has a nonuniform way to display fields. For example, it shows the > >> > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP > >> > packet then it just says "ip" instead, and if it's TCP then it just says > >> > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on. > >> > >> I enjoy a challenge from time to time ;-) > >> > >> Moreover, I'm willing to do the work. Agreed, there's lots of it > >> (dozens is right), but I think it's an elegant solution. > > > > Do you have a basic plan for how to do it? There's definitely room for > > better display here, but I want the code to be clean and maintainable. > > So, every plan that I've gone through starts with a separate code path > for the table approach if I want to stage it. > > It probably makes sense to start there, and add the fields I've been > requested to add, first, and as part of the same series hook up > ovs-ofctl dump-flows to call that new code path. > > Does that make sense? Sure. I'd prefer to make the whole thing a single series because in this case I don't want to add library code that won't be used, but it sounds like you already plan to do it that way.
diff --git a/lib/table.c b/lib/table.c index 9158499..ca92848 100644 --- a/lib/table.c +++ b/lib/table.c @@ -532,6 +532,191 @@ table_print_json__(const struct table *table, const struct table_style *style) free(s); } +/* Dynamic format parser. */ + +struct table_output_action { + struct ds *print_value; /* When !NULL, the text here is output. */ + size_t column_id; /* When print_value is NULL, this is the + * column to print, instead. */ + enum table_format cell_style; /* TF_*. */ +}; + +/* This compiles an input string into a series out output actions for the + * table formatting routines. */ +static size_t +table_print_compile_format(const char *format, + struct table_output_action **output, + const struct table *table) +{ + struct table_output_action *actions; + char escape_vals[UCHAR_MAX] = {0}; + bool escape_on_br = false; + struct ds *action = NULL; + const char *c = format; + bool escape = false; + size_t i = 0; + + if (!format || !table) { + return 0; + } + + actions = xmalloc(sizeof(struct table_output_action)); + + memset(escape_vals, '?', sizeof(escape_vals)); + escape_vals['a'] = '\a'; + escape_vals['b'] = '\b'; + escape_vals['n'] = '\n'; + escape_vals['r'] = '\r'; + escape_vals['t'] = '\t'; + + for (; c; c++) { + if (!action) { + action = xmalloc(sizeof(struct ds)); + ds_init(action); + } + + switch (*c) { + /* default case - accumulate more in the action buffer */ + default: + { + size_t ch = *c; + if (escape) { + if (ch >= sizeof(escape_vals)) { + ch = 0; + } + ch = escape_vals[ch]; + escape = false; + } + + ds_put_char(action, ch); + } + break; + + /* This is a pretty standard 'escape' character. */ + case '\\': + if (!escape) { + escape = true; + } else { + ds_put_char(action, *c); + escape = false; + } + break; + + /* The special state - start a new action block. This printing + * state machine will try to have as few special states as possible, + * so the 'start' state is really just 'complete the previous block' + * state. Unlike another block completion state, this state will + * not create a column lookup action. */ + case '{': + if (escape) { + ds_put_char(action, *c); + escape = false; + escape_on_br = true; + } else { + actions[i++].print_value = action; + action = NULL; + actions = xrealloc(actions, (i+1) * sizeof(actions[0])); + } + break; + + /* The other special state - create a column lookup. See the + * previous comments on the '{' case. This is similar, except it + * does create a column lookup action. */ + case '}': + if (escape || escape_on_br) { + ds_put_char(action, *c); + escape = false; + escape_on_br = false; + } else { + /* allow for ':' to be a special character. */ + struct table_output_action *action_to_modify; + char *ch = strchr(ds_cstr(action), ':'); + enum table_format fmt = TF_TABLE; + + action_to_modify = &actions[i++]; + if (ch) { + struct table_style format_extractor; + *ch++ = 0; + table_parse_format(&format_extractor, ch); + fmt = format_extractor.format; + } + + /* find the header. If not found, we fail. */ + size_t hdrid; + for (hdrid = 0; hdrid < table->n_columns + && strcmp(ds_cstr(action), + table->columns[hdrid].heading); + ++hdrid) { + /* skip forward until we find the header. */ + } + + if (hdrid >= table->n_columns) { + char *end; + hdrid = strtoul(ds_cstr(action), &end, 10); + if (*end != '\0') { + ovs_fatal(0, "Unknown column specified \"%s\"", + ds_cstr(action)); + } + } + + action_to_modify->print_value = NULL; + action_to_modify->cell_style = fmt; + action_to_modify->column_id = hdrid; + actions = xrealloc(actions, (i+1) * sizeof(actions[0])); + ds_clear(action); + break; + } + } + if (!(*c)) { + actions[i++].print_value = action; + break; + } + } + + *output = actions; + return i; +} + +static void +table_print_by_fmt__(const struct table *table, + const struct table_style *style) +{ + struct table_output_action *actions; + size_t n_actions; + + n_actions = table_print_compile_format(style->format_string, &actions, + table); + + for (size_t row = 0; row < table->n_rows; ++row) { + for (size_t action = 0; action < n_actions; ++action) { + if (actions[action].print_value) { + fputs(ds_cstr(actions[action].print_value), stdout); + } else { + struct cell *cell = table_cell__(table, row, + actions[action].column_id); + const char *str = cell_to_text(cell, style); + + switch (actions[action].cell_style) + { + case TF_TABLE: + case TF_LIST: + case TF_JSON: + case TF_FORMATTED: + default: + fputs(str, stdout); + break; + case TF_CSV: + table_print_csv_cell__(str); + break; + case TF_HTML: + table_escape_html_text__(str, strlen(str)); + break; + } + } + } + } +} + /* Parses 'format' as the argument to a --format command line option, updating * 'style->format'. */ void @@ -547,6 +732,9 @@ table_parse_format(struct table_style *style, const char *format) style->format = TF_CSV; } else if (!strcmp(format, "json")) { style->format = TF_JSON; + } else if (strchr(format, '{')) { + style->format = TF_FORMATTED; + style->format_string = xstrdup(format); } else { ovs_fatal(0, "unknown output format \"%s\"", format); } @@ -592,5 +780,9 @@ table_print(const struct table *table, const struct table_style *style) case TF_JSON: table_print_json__(table, style); break; + + case TF_FORMATTED: + table_print_by_fmt__(table, style); + break; } } diff --git a/lib/table.h b/lib/table.h index 85b8156..0880273 100644 --- a/lib/table.h +++ b/lib/table.h @@ -64,7 +64,8 @@ enum table_format { TF_LIST, /* One cell per line, one row per paragraph. */ TF_HTML, /* HTML table. */ TF_CSV, /* Comma-separated lines. */ - TF_JSON /* JSON. */ + TF_JSON, /* JSON. */ + TF_FORMATTED /* A format string was specified. */ }; enum cell_format { @@ -78,9 +79,10 @@ struct table_style { enum cell_format cell_format; /* CF_*. */ bool headings; /* Include headings? */ int json_flags; /* CF_JSON: Flags for json_to_string(). */ + char *format_string; /* Valid only with TF_FORMATTED format. */ }; -#define TABLE_STYLE_DEFAULT { TF_TABLE, CF_STRING, true, JSSF_SORT } +#define TABLE_STYLE_DEFAULT { TF_TABLE, CF_STRING, true, JSSF_SORT, NULL } #define TABLE_OPTION_ENUMS \ OPT_NO_HEADINGS, \ diff --git a/lib/table.man b/lib/table.man index a8f1094..d47fc71 100644 --- a/lib/table.man +++ b/lib/table.man @@ -32,6 +32,23 @@ that represent OVSDB data or data types are expressed in the format described in the OVSDB specification; other cells are simply expressed as text strings. .RE +.IP "\fBFORMAT\fR" +\fBFORMAT\fR will emit each row of the table according to the format +string specified. The format string is comprised of either literal +text, or replacement fields delimited by braces \fB{\fIfield\fR}\fR. +Fields may be either a numeric value (which will be the ordinal +position of the corresponding heading, index starting at 0), or the +heading name. +Additionally, the cell data emitted can be modified with an additional +modification hint, corresponding to a cell output format. To specify +the hint, simply put a ':' after the heading, followed by the cell +format string. +.RS +\fIExample\fR: \fB\-f "elemt0={0:json},elemt1={heading3}"\fR will emit +rows as "elemt0=" followed by the cell corresponding to the first +heading in json format, followed by ",elemt1=", followed by the cell +corresponding to "heading3" +.RE .RE . .IP "\fB\-d \fIformat\fR"
Adds a new --format="xx={0},..." to the table library, which allows users to make table output formats to match their own needs. Signed-off-by: Aaron Conole <aconole@redhat.com> --- v1 url: https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325235.html TODO: Still needs unit tests to check the format strings One thing I want to do is put in the idea of string length to the grammar. Probably will be a few other additions (ex: text which is omitted when the corresponding column value is empty). NOTE: Submitted for early feedback lib/table.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/table.h | 6 +- lib/table.man | 17 ++++++ 3 files changed, 213 insertions(+), 2 deletions(-)