Re: [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Laszlo,

I actually prefer the one arg per line in the call
to match the function prototype that requires one
arg per line.

Since most of the code follows this one arg per line
convention today, I would not be in favor of a 
non-backwards compatible change to the coding style.

Either leave it alone.  Or if there is a really strong
desire to put multiple args in a single line then 
document that as an optional supported style with the
preferred style being one arg per line.

Mike

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, August 11, 2017 1:45 PM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
> devel@lists.01.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Leif
> Lindholm <leif.lindholm@linaro.org>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [edk2-CCodingStandardsSpecification PATCH
> 2/2] Source Files / Spacing / Multi-line func. calls: allow
> condensed arguments
> 
> On 2017-08-11 09:48:51, Laszlo Ersek wrote:
> > The "one argument per line" style as the sole possibility
> takes up too
> > much vertical space, wastes perfectly good horizontal space,
> and causes a
> > constant jump-to-the-left eye movement for the reader.
> >
> > Now that we have limited the line length to 80 colums, the
> "condensed
> > arguments" style cannot be abused; permit it.
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >  5_source_files/52_spacing.md | 29 +++++++++++++++++++-
> >  README.md                    |  1 +
> >  2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/5_source_files/52_spacing.md
> b/5_source_files/52_spacing.md
> > index ddeabf7753a8..fc8a5e7e58e7 100644
> > --- a/5_source_files/52_spacing.md
> > +++ b/5_source_files/52_spacing.md
> > @@ -133,7 +133,9 @@ If ((--MyInteger) > 0) {
> >  #### 5.2.2.4 Subsequent lines of multi-line function calls
> should line up two spaces from the beginning of the function
> name
> >
> >  If a function call or function like macro invocation is
> broken up into multiple
> > -lines, then:
> > +lines, then follow one of the alternatives below.
> > +
> > +##### 5.2.2.4.1 The "one argument per line" style
> >
> >  * One argument per line, including the first argument on its
> own line.
> >  * Indent each argument 2 spaces from the start of the
> function name. If a
> > @@ -165,6 +167,31 @@ DEBUG ((
> >    ));
> >  ```
> >
> > +Use this line breaking style especially if it saves a format
> string or complex
> > +argument from being split, or when commenting on individual
> arguments.
> > +
> > +##### 5.2.2.4.2 The "condensed arguments" style
> > +
> > +For most function calls and function-like macro invocations,
> the "one argument
> > +per line" style uses up valuable vertical space without
> utilizing readily
> > +available horizontal space. Such statements are permitted to
> condense the
> > +arguments and the closing parenthesis (or parentheses), up to
> the allowed line
> > +length. The indentation requirements are identical to those
> of the "one
> > +argument per line" style.
> > +
> > +```c
> > +CopyMem (Destination, Source, SIZE_4KB);
> > +
> > +Status = gBS->AllocatePool (EfiBootServicesData, sizeof
> (DRIVER_NAME_INSTANCE),
> > +                &PrivateData);
> 
> I prefer that we just have one style, and just drop the
> requirement
> that multiline param lists can only have one arg per line. I
> think it
> is good to have the first param start on the next line in this
> case.
> 
>   Status = gBS->AllocatePool (
>                   EfiBootServicesData, sizeof
> (DRIVER_NAME_INSTANCE),
>                   &PrivateData);
> 
> This compacts things somewhat, but still keeps the parameter
> list
> aligned horizontally on subsequent lines.
> 
> Regarding the closing parens being on a separate line, I think
> we
> should also remove that as a requirement.
> 
> I think you might want to break out the multiple params per line
> and
> closing parens change out to allow for separate discussion.
> 
> -Jordan
> 
> > +
> > +DEBUG ((DEBUG_INFO, "The addresses of the 4 buffers are %p,
> %p, %p, and %p\n",
> > +  Buffer1, Buffer2, Buffer3, Buffer4));
> > +```
> > +
> > +This line breaking style prevents overly frequent saccades to
> the left, without
> > +resulting in overlong lines.
> > +
> >  #### 5.2.2.5 Always put space after commas or semicolons that
> separate items
> >
> >  This punctuation is not necessary if no code or comments
> follow the comma or
> > diff --git a/README.md b/README.md
> > index 8fad5a327b8c..437024e57677 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -113,3 +113,4 @@ Copyright (c) 2006-2017, Intel
> Corporation. All rights reserved.
> >  | 2.2      | Convert to Gitbook
> | June 2017  |
> >  |          |
> [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS]
> clarify line breaking and indentation requirements for multi-
> line function calls |            |
> >  |          | Limit lines to 80 columns
> |            |
> > +|          | Introduce the "condensed arguments" line
> breaking style
> |            |
> > --
> > 2.13.1.3.g8be5a757fa67
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel