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 <[email protected]>
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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
