Re: [ovs-dev] [PATCH v9 3/6] appctl: Add option '--pretty' for pretty-printing JSON output.

2024-05-02 Thread Ilya Maximets
On 4/12/24 09:26, jm...@redhat.com wrote:
> From: Jakob Meng 
> 
> 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 
> ---
>  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(_argv)) {
>  error = unixctl_client_transact(client, "set-options",
>  opt_argv.n, opt_argv.names,
> -args->format, _result,
> -_error);
> +args->format, 0,
> +_result, _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,
> -   

[ovs-dev] [PATCH v9 3/6] appctl: Add option '--pretty' for pretty-printing JSON output.

2024-04-12 Thread jmeng
From: Jakob Meng 

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 
---
 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)
 {
 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(_argv)) {
 error = unixctl_client_transact(client, "set-options",
 opt_argv.n, opt_argv.names,
-args->format, _result,
-_error);
+args->format, 0,
+_result, _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, _result, _error);
+args->format, args->format_flags,
+_result, _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\