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

Reply via email to