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

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-08-11 14:04:21, Kinney, Michael D wrote:
> Laszlo,
> 
> I actually prefer the one arg per line in the call
> to match the function prototype that requires one
> arg per line.

One per line on prototypes seems reasonable.

When it come to calling, there are potentially many calls per function
prototype/definition.

The rule for calls is inconsistent regarding if one or more than one
line is used. If the parameters fit on one line, then it is okay to
have multiple params on a line. Why is it okay for the single line
case, if not for the multiple line case?

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

I agree, which is why I suggest we let multiple params appear per
line, but if multiple lines are used, no params should appear on the
first line with the function name. Therefore the old code follows the
style. It simply breaks the params up onto more lines.

Regarding the separate line for ending the function call: ');' If we
make this optional, then the old code will also still follow the
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.

Would my suggestion be an ok compromise that would allow the old style
to still be used?

I don't know about stating that there is a preference for a single
line per param. Do we need to feel bad if we write or accept patches
that don't follow this style? :) If we state that the project has a
preference, then it doesn't seem like personal preference should be
enough of a reason to go against the project preference.

-Jordan

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