Libguestfs - Apr 2023 - [libnbd PATCH 3/3] generator: indent C argument list 2 spaces relative to function designator

Change the optional "parens" parameter of "print_arg_list"
from bool to a
new union type. The new union type has three data constructors, NoParens
(matching the previous "false" value), ParensSameLine (matching the
previous "true" value), and a third constructor called
"ParensNewLine",
which wraps an "int". [ParensNewLine n] stands for the following
formatting construct (with wrapping):
> ret_type foobar (
>            type1 param1, type2 param2, type3 param3, type4 param4,
>            type5 param5
>          );
> <---n--->
and without wrapping:
> ret_type foobar (
>            type1 param1, type2 param2, type3 param3, type4 param4, type5
param5
>          );
> <---n--->
Capture "n" (that is, the 0-based column where the function designator
starts) in "print_call", and put the new formatting to use.

In effect, still utilize "pr_wrap", for wrapping the argument list at
comma (",") characters, but shift the left hand side of the wrapped
block
from the end of the function designator near the front of it.

The use case is: if "foobar" is long, and at least one
"typeN" is also
long, previously we would get
> ret_type extremely_long_verbose_foobar (extremely_long_verbose_type1
param1,
>                                         type2 param2, type3 param3,
>                                         type4 param4, type5 param5);
That is, the first wrapping opportunity on a particular line would be too
late already, making the source code too wide.

This patch changes the generated files "include/libnbd.h" and
"lib/unlocked.h". Taking the declaration responsible for the longest
line
in each of those header files, the declaration is reformatted as shown
below:

[include/libnbd.h]
> -extern int nbd_aio_opt_list_meta_context_queries (struct nbd_handle *h,
> -                                                  char **queries,
> -                                                  nbd_context_callback
context_callback,
> -                                                  nbd_completion_callback
completion_callback)
> +extern int nbd_aio_opt_list_meta_context_queries (
> +             struct nbd_handle *h, char **queries,
> +             nbd_context_callback context_callback,
> +             nbd_completion_callback completion_callback
> +           )
>      LIBNBD_ATTRIBUTE_NONNULL (1, 2);
[lib/unlocked.h]
> -extern int nbd_unlocked_aio_opt_list_meta_context_queries (struct
nbd_handle *h,
> -                                                           char **queries,
> -                                                          
nbd_context_callback *context_callback,
> -                                                          
nbd_completion_callback *completion_callback)
> +extern int nbd_unlocked_aio_opt_list_meta_context_queries (
> +             struct nbd_handle *h, char **queries,
> +             nbd_context_callback *context_callback,
> +             nbd_completion_callback *completion_callback
> +           )
>      LIBNBD_ATTRIBUTE_NONNULL (1, 2);
Update the GoLang generator to stick with "no parens".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 generator/C.mli     |  3 +-
 generator/C.ml      | 33 ++++++++++++++++----
 generator/GoLang.ml |  4 +--
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/generator/C.mli b/generator/C.mli
index 215acc64f8a9..e773d4108bf9 100644
--- a/generator/C.mli
+++ b/generator/C.mli
@@ -18,6 +18,7 @@
  *)
 
 type closure_style = Direct | AddressOf | Pointer
+type args_style = NoParens | ParensSameLine | ParensNewLine of int
 
 val generate_lib_libnbd_syms : unit -> unit
 val generate_include_libnbd_h : unit -> unit
@@ -28,7 +29,7 @@ val
 val generate_docs_api_flag_links_pod : unit -> unit
 val generate_docs_nbd_pod : string -> API.call -> unit -> unit
 val print_arg_list : ?wrap:bool -> ?maxcol:int ->
-                     ?handle:bool -> ?types:bool -> ?parens:bool ->
+                     ?handle:bool -> ?types:bool -> ?parens:args_style
->
                      ?closure_style:closure_style ->
                      API.arg list -> API.optarg list -> unit
 val print_cbarg_list : ?wrap:bool -> ?maxcol:int ->
diff --git a/generator/C.ml b/generator/C.ml
index 044c046dcff1..9815d690fe2c 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -23,6 +23,7 @@
 open Utils
 
 type closure_style = Direct | AddressOf | Pointer
+type args_style = NoParens | ParensSameLine | ParensNewLine of int
 
 let generate_lib_libnbd_syms ()    generate_header HashStyle;
@@ -134,15 +135,32 @@ let
 
 let optarg_attr_nonnull (OClosure _ | OFlags _) = [ false ]
 
-let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types ?(parens = true)
-          ?closure_style args optargs -  if parens then pr "(";
+let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types
+          ?(parens = ParensSameLine) ?closure_style args optargs +  (match
parens with
+   | NoParens -> ()
+   | ParensSameLine -> pr "("
+   | ParensNewLine indent ->
+       pr "(\n";
+       for i = 0 to indent + 2 - 1 do
+         pr " "
+       done
+  );
   if wrap then
     pr_wrap ?maxcol ','
       (fun () -> print_arg_list' ?handle ?types ?closure_style args
optargs)
   else
     print_arg_list' ?handle ?types ?closure_style args optargs;
-  if parens then pr ")"
+  (match parens with
+   | NoParens -> ()
+   | ParensSameLine -> pr ")"
+   | ParensNewLine indent ->
+       pr "\n";
+       for i = 0 to indent - 1 do
+         pr " "
+       done;
+       pr ")"
+  )
 
 and print_arg_list' ?(handle = false) ?(types = true) ?(closure_style =
Direct)
           args optargs @@ -237,8 +255,11 @@ and
   ) optargs
 
 let print_call ?wrap ?maxcol ?closure_style name args optargs ret -  pr
"%s nbd_%s " (type_of_ret ret) name;
-  print_arg_list ~handle:true ?wrap ?maxcol ?closure_style args optargs
+  pr "%s " (type_of_ret ret);
+  let designator_column = output_column () in
+  pr "nbd_%s " name;
+  print_arg_list ~handle:true ?wrap ?maxcol
+      ~parens:(ParensNewLine designator_column) ?closure_style args optargs
 
 let print_fndecl ?wrap ?closure_style name args optargs ret    pr "extern
";
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 3120b7f7c72f..4ab6b2626ae3 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -135,7 +135,7 @@ let
   pr "%s\n" ret_c_type;
   pr "_nbd_%s_wrapper (struct error *err,\n" name;
   pr "        ";
-  C.print_arg_list ~wrap:true ~handle:true ~parens:false args optargs;
+  C.print_arg_list ~wrap:true ~handle:true ~parens:NoParens args optargs;
   pr ")\n";
   pr "{\n";
   pr "#ifdef LIBNBD_HAVE_NBD_%s\n" ucname;
@@ -736,7 +736,7 @@ let
       let ret_c_type = C.type_of_ret ret in
       pr "%s _nbd_%s_wrapper (struct error *err,\n" ret_c_type name;
       pr "        ";
-      C.print_arg_list ~wrap:true ~handle:true ~parens:false args optargs;
+      C.print_arg_list ~wrap:true ~handle:true ~parens:NoParens args optargs;
       pr ");\n";
   ) handle_calls;
   pr "\n";

On Thu, Apr 20, 2023 at 04:04:58PM +0200, Laszlo Ersek
wrote:
> Change the optional "parens" parameter of
"print_arg_list" from bool to a
> new union type. The new union type has three data constructors, NoParens
> (matching the previous "false" value), ParensSameLine (matching
the
> previous "true" value), and a third constructor called
"ParensNewLine",
> which wraps an "int". [ParensNewLine n] stands for the following
> formatting construct (with wrapping):
> 
> > ret_type foobar (
> >            type1 param1, type2 param2, type3 param3, type4 param4,
> >            type5 param5
> >          );
> > <---n--->
I would have called it ParensNewLineWithIndent, and I would
have called this:
> +type args_style = NoParens | ParensSameLine | ParensNewLine of int
parens_style or parens_t just to tie it back to the only parameter
where it is used.
> +let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types
> +          ?(parens = ParensSameLine) ?closure_style args optargs > + 
(match parens with
> +   | NoParens -> ()
> +   | ParensSameLine -> pr "("
> +   | ParensNewLine indent ->
> +       pr "(\n";
> +       for i = 0 to indent + 2 - 1 do
> +         pr " "
> +       done
libguestfs common/mlstdutils/std_utils.ml has a "spaces" function
which might be worth grabbing for this.  The license is compatible
enough, and if not as author I give you permission.
>    if wrap then
>      pr_wrap ?maxcol ','
>        (fun () -> print_arg_list' ?handle ?types ?closure_style args
optargs)
>    else
>      print_arg_list' ?handle ?types ?closure_style args optargs;
> -  if parens then pr ")"
> +  (match parens with
> +   | NoParens -> ()
> +   | ParensSameLine -> pr ")"
> +   | ParensNewLine indent ->
> +       pr "\n";
> +       for i = 0 to indent - 1 do
> +         pr " "
> +       done;
> +       pr ")"
> +  )
& here.

The rest looks good to me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top