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

Laszlo Ersek posted 2 patches 7 years, 4 months ago
[edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Laszlo Ersek 7 years, 4 months ago
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);
+
+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
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Jordan Justen 7 years, 4 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Kinney, Michael D 7 years, 4 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Jordan Justen 7 years, 4 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Andrew Fish 7 years, 4 months ago
> On Aug 11, 2017, at 1:45 PM, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 
> 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);
> 

I like using as much width as allowed, but then again I've got 3440 X 1440 monitors I'm using all the time. 

Another place that multiple things on the same line make sense is InstallMultipleProtocolInterfaces. I think that makes the code more readable. 

Status - gBS->InstallMultipleProtocolInterfaces (
		NULL,
                &gEdk2Protocol1Guid, 	                        &gEdk2Protocol1Data, 
                &gEdk2Protocol2WithLongNameGuid, 	&gEdk2Protocol2WithLongNameData, 
                NULL
                );

Given a lot of this stuff is arbitrary (I had fun on the 1st coding standard doc) I think staying with what a lot of the code already does is not a bad idea.

Thanks,

Andrew Fish


> 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Leif Lindholm 7 years, 4 months ago
On Fri, Aug 11, 2017 at 01:45:12PM -0700, Jordan Justen wrote:
> > +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);

I like this one better than Laszlo's proposal, since it keeps the
start of each line of parameters in the same column.

> 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 would also support that change.

> I think you might want to break out the multiple params per line and
> closing parens change out to allow for separate discussion.

That would make sense.

Laszlo - many thanks to starting this discussion!

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Posted by Laszlo Ersek 7 years, 4 months ago
On 08/12/17 12:13, Leif Lindholm wrote:
> On Fri, Aug 11, 2017 at 01:45:12PM -0700, Jordan Justen wrote:
>>> +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);
> 
> I like this one better than Laszlo's proposal, since it keeps the
> start of each line of parameters in the same column.
> 
>> 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 would also support that change.
> 
>> I think you might want to break out the multiple params per line and
>> closing parens change out to allow for separate discussion.
> 
> That would make sense.
> 
> Laszlo - many thanks to starting this discussion!

Thanks all for the feedback. Before I attempt to post version 2, I'd
like to see the "preference" wording sorted out (raised by Mike and
Jordan). I.e., whether the new style should be merely tolerated (but
still frowned upon) or if it should be a first class citizen.

(Stating the obvious,) if the new style were accepted into the CCS,
personally I would switch to it for all new code written by me (even if
the code were for MdePkg or MdeModulePkg), independently of the
preference assigned to the new style.

I would not require OvmfPkg / ArmVirtPkg contributors to follow my
preference -- NB I've been enforcing the one style we now have in the
CCS, for the packages I co-maintain, in spite of my dislike for that
style -- but I might suggest the alternative style to others if I
perceived the vertical space consumption very disturbing.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel