On Mon, Feb 15, 2016 at 04:22:07PM +0100, Quentin Monnet wrote: > Add color output for flow actions for ovs-ofctl dump-flows command > utility, by calling ds_put_color() instead of ds_put_format() in the > functions responsible for printing the actions. > > This also causes the option to be propagated upward to the other callers > of those functions (partially implemented, to be completed if colors are > to be provided for other commands / tools). > > Signed-off-by: Quentin Monnet <quentin.mon...@6wind.com>
At a skim, I'm OK with the rest of this series. It doesn't apply at the moment due to patch rejects, but that can be fixed. I have a suggestion that might make the code easier to read. I haven't tried it so I'm not sure. It's inconvenient to have to use functions to emit color markers, either by calling both ds_put_color_start() and ds_put_color_end() or by calling ds_put_color(). However, these functions always emit a fixed string for any given color_option and sgr_seq. It seems like it would be possible, then, to put all of these into a struct, like this: struct colors { /* Color codes for various situation. Each of these is a fully formed * SGR start string for the appropriate color. */ char *actions; char *drop; char *learn; char *param; char *paren; char *special; char *value; /* SGR end string. */ char *end; }; Then it becomes easier to emit whatever colors ones want. Just pass a "struct colors" into the function that might want to emit color, and it can go ahead and do things like this: ds_put_format(string, " %scookie=%s0x%"PRIx64, colors->param, colors->end, ntohll(fs->cookie)); If the caller doesn't actually want color, then it can pass in a struct colors whose members are all "". _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev