On 4/12/24 09:26, [email protected] wrote:
> From: Jakob Meng <[email protected]>
> 
> With the '--pretty' option, ovs-appctl will now print JSON output in a
> more readable fashion, i.e. with additional line breaks, spaces and
> sorted dictionary keys.
> 
> Signed-off-by: Jakob Meng <[email protected]>
> ---
>  NEWS                   |  3 +++
>  lib/unixctl.c          |  6 +++---
>  lib/unixctl.h          |  2 +-
>  tests/ovs-vswitchd.at  |  5 +++++
>  utilities/ovs-appctl.c | 32 ++++++++++++++++++++++++++++----
>  5 files changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index f18238159..52cadbe0d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,9 @@ Post-v3.3.0
>     - ovs-appctl:
>       * Added new option [-f|--format] to choose the output format, e.g. 
> 'json'
>         or 'text' (by default).
> +     * Added new option [--pretty] to print JSON output in a readable 
> fashion.
> +       E.g. members of objects and elements of arrays are printed one per 
> line,
> +       with indentation.
>     - Python:
>       * Added support for choosing the output format, e.g. 'json' or 'text'.
>  
> diff --git a/lib/unixctl.c b/lib/unixctl.c
> index 18638d86a..67cf26418 100644
> --- a/lib/unixctl.c
> +++ b/lib/unixctl.c
> @@ -572,8 +572,8 @@ unixctl_client_create(const char *path, struct jsonrpc 
> **client)
>   * '*err' if not NULL. */
>  int
>  unixctl_client_transact(struct jsonrpc *client, const char *command, int 
> argc,
> -                        char *argv[], enum ovs_output_fmt fmt, char **result,
> -                        char **err)
> +                        char *argv[], enum ovs_output_fmt fmt,
> +                        unsigned int fmt_flags, char **result, char **err)

As I mentioned in the review for patch #1, it may be better to handle
for matting in the ovs-appctl instead and keep unixctl module unaware
of it.

>  {
>      struct jsonrpc_msg *request, *reply;
>      struct json **json_args, *params, *output_src;
> @@ -604,7 +604,7 @@ unixctl_client_transact(struct jsonrpc *client, const 
> char *command, int argc,
>          if (fmt == OVS_OUTPUT_FMT_TEXT && output_src->type == JSON_STRING) {
>              *output_dst = xstrdup(json_string(output_src));
>          } else if (fmt == OVS_OUTPUT_FMT_JSON) {
> -            *output_dst = json_to_string(output_src, 0);
> +            *output_dst = json_to_string(output_src, fmt_flags);
>          } else {
>              VLOG_WARN("%s: unexpected %s type in JSON rpc reply: %s",
>                        jsonrpc_get_name(client),
> diff --git a/lib/unixctl.h b/lib/unixctl.h
> index b30bcf092..5a08a1bd1 100644
> --- a/lib/unixctl.h
> +++ b/lib/unixctl.h
> @@ -39,7 +39,7 @@ int unixctl_client_create(const char *path, struct jsonrpc 
> **client);
>  int unixctl_client_transact(struct jsonrpc *client,
>                              const char *command,
>                              int argc, char *argv[],
> -                            enum ovs_output_fmt fmt,
> +                            enum ovs_output_fmt fmt, unsigned int fmt_flags,
>                              char **result, char **error);
>  
>  /* Command registration. */
> diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
> index 32ca2c6e7..5683896ef 100644
> --- a/tests/ovs-vswitchd.at
> +++ b/tests/ovs-vswitchd.at
> @@ -275,4 +275,9 @@ ovs_version=$(ovs-appctl version)
>  AT_CHECK_UNQUOTED([ovs-appctl --format json version], [0], [dnl
>  {"reply-format":"plain","reply":"$ovs_version\n"}])
>  
> +AT_CHECK_UNQUOTED([ovs-appctl --format json --pretty version], [0], [dnl
> +{
> +  "reply": "$ovs_version\n",
> +  "reply-format": "plain"}])
> +
>  AT_CLEANUP
> diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
> index 4d4503597..29a0de2ee 100644
> --- a/utilities/ovs-appctl.c
> +++ b/utilities/ovs-appctl.c
> @@ -26,6 +26,7 @@
>  #include "daemon.h"
>  #include "dirs.h"
>  #include "openvswitch/dynamic-string.h"
> +#include "openvswitch/json.h"
>  #include "jsonrpc.h"
>  #include "process.h"
>  #include "timeval.h"
> @@ -39,6 +40,7 @@ static void usage(void);
>  /* Parsed command line args. */
>  struct cmdl_args {
>      enum ovs_output_fmt format;
> +    unsigned int format_flags;
>      char *target;
>  };
>  
> @@ -74,8 +76,8 @@ main(int argc, char *argv[])
>      if (!svec_is_empty(&opt_argv)) {
>          error = unixctl_client_transact(client, "set-options",
>                                          opt_argv.n, opt_argv.names,
> -                                        args->format, &cmd_result,
> -                                        &cmd_error);
> +                                        args->format, 0,
> +                                        &cmd_result, &cmd_error);
>  
>          if (error) {
>              ovs_fatal(error, "%s: transaction error", args->target);
> @@ -98,7 +100,9 @@ main(int argc, char *argv[])
>      cmd_argc = argc - optind;
>      cmd_argv = cmd_argc ? argv + optind : NULL;
>      error = unixctl_client_transact(client, cmd, cmd_argc, cmd_argv,
> -                                    args->format, &cmd_result, &cmd_error);
> +                                    args->format, args->format_flags,
> +                                    &cmd_result, &cmd_error);
> +
>      if (error) {
>          ovs_fatal(error, "%s: transaction error", args->target);
>      }
> @@ -144,6 +148,11 @@ Other options:\n\
>    --timeout=SECS     wait at most SECS seconds for a response\n\
>    -f, --format=FMT   Output format. One of: 'json', or 'text'\n\
>                       ('text', by default)\n\
> +  --pretty           By default, JSON in output is printed as compactly as\n\
> +                     possible. This option causes JSON in output to be\n\
> +                     printed in a more readable fashion. Members of 
> objects\n\
> +                     and elements of arrays are printed one per line, with\n\
> +                     indentation.\n\

Too many words.  The full description should be in a man page.
Here we only need a very brief one, e.g. "Format the output in
a more readable fashion.  Requires: --format=json." or something
like this.

>    -h, --help         Print this helpful information\n\
>    -V, --version      Display ovs-appctl version information\n",
>             program_name, program_name);
> @@ -155,6 +164,7 @@ cmdl_args_create(void) {
>      struct cmdl_args *args = xmalloc(sizeof *args);
>  
>      args->format = OVS_OUTPUT_FMT_TEXT;
> +    args->format_flags = 0;
>      args->target = NULL;
>  
>      return args;
> @@ -171,7 +181,8 @@ parse_command_line(int argc, char *argv[])
>  {
>      enum {
>          OPT_START = UCHAR_MAX + 1,
> -        VLOG_OPTION_ENUMS
> +        OPT_PRETTY,
> +        VLOG_OPTION_ENUMS,
>      };
>      static const struct option long_options[] = {
>          {"target", required_argument, NULL, 't'},
> @@ -179,6 +190,7 @@ parse_command_line(int argc, char *argv[])
>          {"format", required_argument, NULL, 'f'},
>          {"help", no_argument, NULL, 'h'},
>          {"option", no_argument, NULL, 'o'},
> +        {"pretty", no_argument, NULL, OPT_PRETTY},
>          {"version", no_argument, NULL, 'V'},
>          {"timeout", required_argument, NULL, 'T'},
>          VLOG_LONG_OPTIONS,
> @@ -188,6 +200,7 @@ parse_command_line(int argc, char *argv[])
>      char *short_options = xasprintf("+%s", short_options_);
>      struct cmdl_args *args = cmdl_args_create();
>      unsigned int timeout = 0;
> +    bool pretty = false;
>      int e_options;
>  
>      e_options = 0;
> @@ -230,6 +243,10 @@ parse_command_line(int argc, char *argv[])
>              ovs_cmdl_print_options(long_options);
>              exit(EXIT_SUCCESS);
>  
> +        case OPT_PRETTY:
> +            pretty = true;
> +            break;
> +
>          case 'T':
>              if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
>                  ovs_fatal(0, "value %s on -T or --timeout is invalid", 
> optarg);
> @@ -259,6 +276,13 @@ parse_command_line(int argc, char *argv[])
>                    "(use --help for help)");
>      }
>  
> +    if (pretty) {
> +        if (args->format != OVS_OUTPUT_FMT_JSON) {
> +            ovs_fatal(0, "--pretty is supported with --format json only");
> +        }
> +        args->format_flags |= JSSF_PRETTY | JSSF_SORT;

Should likley always sort, regardless of 'pretty'.

> +    }
> +
>      if (!args->target) {
>          args->target = xstrdup("ovs-vswitchd");
>      }

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to