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

[Laszlo Ersek]

On 4/20/23 16:36, Richard W.M. Jones wrote:
> 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.

I'll adopt these, thanks!

> 
>> +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.

Well I took the open-coded loop directly from the "pr_wrap"
implementation [generator/utils.ml] :)

...
         for i = 0 to wrapping_col-1 do pr " " done;
...

So I guess I should rebase that to the "spaces" function too.

Now, looking up what "spaces" does...

    let spaces n = String.make n ' '

OK, that's simple enough. (But, I agree it adds value: "spaces 5" is
much easier to understand than "String.make 5 ' '".)

> 
> The rest looks good to me.

Thanks!
Laszlo