[edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[Laszlo Ersek]

We've discussed these ideas repeatedly over time; I'm now attempting to
formalize them. Also test-driving the documentation contribution
process.

- Repo: https://github.com/lersek/edk2-CCodingStandardsSpecification.git
- Branch: line_wrapping
- Rendered views of the pages modified:
  - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/line_wrapping/#edk-ii-c-coding-standards-specification
  - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/line_wrapping/5_source_files/#51-general-rules
  - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-coding-standards-sp/content/v/line_wrapping/5_source_files/52_spacing.html#52-spacing

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>

Thanks
Laszlo

Laszlo Ersek (2):
  Source Files / General Rules: limit line lengths to 80 columns
  Source Files / Spacing / Multi-line func. calls: allow condensed
    arguments

 5_source_files/52_spacing.md | 29 +++++++++++++++++++-
 5_source_files/README.md     | 17 ++++++++----
 README.md                    |  2 ++
 3 files changed, 42 insertions(+), 6 deletions(-)

-- 
2.13.1.3.g8be5a757fa67

[edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Laszlo Ersek]

We currently say "stick with 80 if it's convenient, extend to 120
otherwise". This is too lax; much new edk2 code ignores the 80 columns
recommendation, resulting in source files that are hard to read for some
contributors. Remove the 120 columns excuse and make 80 columns a
requirement.

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/README.md | 17 ++++++++++++-----
 README.md                |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/5_source_files/README.md b/5_source_files/README.md
index a93492db4f0f..546d44d94fcb 100644
--- a/5_source_files/README.md
+++ b/5_source_files/README.md
@@ -33,12 +33,19 @@
 
 ## 5.1 General Rules
 
-### 5.1.1 Lines shall be 120 columns, or less
+### 5.1.1 Lines shall be 80 columns, or less
 
-Preferably, limit line lengths to 80 columns or less. When this doesn't leave
-sufficient space for a good postfix style comment, extend the line to a total
-of 120 columns. Having some level of uniformity in the expected width of the
-source is useful for viewing and printing the code.
+Limit line lengths to 80 columns.
+
+Lines longer than 80 columns make it more difficult for the reader to find the
+beginning of the next line. They also tend to prevent users from displaying two
+source listings side-by-side on common display devices.
+
+When the 80 columns limit doesn't leave sufficient space for a postfix style
+comment, break the line into shorter segments at logical boundaries (for
+example, between the arguments of a function call, adhering to the spacing
+rules), or replace the postfix style comment with a standalone comment that
+precedes the statement.
 
 ### 5.1.2 Do not use tab characters
 
diff --git a/README.md b/README.md
index 8b9675b94937..8fad5a327b8c 100644
--- a/README.md
+++ b/README.md
@@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
 | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
 | 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                                                                                                                         |            |
-- 
2.13.1.3.g8be5a757fa67

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Jordan Justen]

On 2017-08-11 09:48:50, Laszlo Ersek wrote:
> We currently say "stick with 80 if it's convenient, extend to 120
> otherwise". This is too lax; much new edk2 code ignores the 80 columns
> recommendation, resulting in source files that are hard to read for some
> contributors. Remove the 120 columns excuse and make 80 columns a
> requirement.
> 
> 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/README.md | 17 ++++++++++++-----
>  README.md                |  1 +
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/5_source_files/README.md b/5_source_files/README.md
> index a93492db4f0f..546d44d94fcb 100644
> --- a/5_source_files/README.md
> +++ b/5_source_files/README.md
> @@ -33,12 +33,19 @@
>  
>  ## 5.1 General Rules
>  
> -### 5.1.1 Lines shall be 120 columns, or less
> +### 5.1.1 Lines shall be 80 columns, or less
>  
> -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
> -sufficient space for a good postfix style comment, extend the line to a total
> -of 120 columns. Having some level of uniformity in the expected width of the
> -source is useful for viewing and printing the code.
> +Limit line lengths to 80 columns.
> +
> +Lines longer than 80 columns make it more difficult for the reader to find the
> +beginning of the next line. They also tend to prevent users from displaying two
> +source listings side-by-side on common display devices.
> +
> +When the 80 columns limit doesn't leave sufficient space for a postfix style
> +comment, break the line into shorter segments at logical boundaries (for
> +example, between the arguments of a function call, adhering to the spacing
> +rules), or replace the postfix style comment with a standalone comment that
> +precedes the statement.
>  
>  ### 5.1.2 Do not use tab characters
>  
> diff --git a/README.md b/README.md
> index 8b9675b94937..8fad5a327b8c 100644
> --- a/README.md
> +++ b/README.md
> @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
>  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
>  | 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                                                                                                                         |            |

Prompting the question as to whether the markdown for this table could
reasonably be expressed in less that 173 columns? :)

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Kinney, Michael D]

Jordan,

MD tables work really well for small tables with short content
in each column. 

Complex tables, and tables with sentences/paragraphs are 
a challenge in MD.

I have considered changing the Revision History from a table
to a list to address this long line issue.

The only other option I have found is to convert the tables
to HTML, which means mixed MD and HTML content, and I am trying
to avoid that.

Mike
 
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, August 11, 2017 1:53 PM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
> devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-CCodingStandardsSpecification PATCH 1/2]
> Source Files / General Rules: limit line lengths to 80 columns
> 
> On 2017-08-11 09:48:50, Laszlo Ersek wrote:
> > We currently say "stick with 80 if it's convenient, extend to
> 120
> > otherwise". This is too lax; much new edk2 code ignores the 80
> columns
> > recommendation, resulting in source files that are hard to
> read for some
> > contributors. Remove the 120 columns excuse and make 80
> columns a
> > requirement.
> >
> > 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/README.md | 17 ++++++++++++-----
> >  README.md                |  1 +
> >  2 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/5_source_files/README.md
> b/5_source_files/README.md
> > index a93492db4f0f..546d44d94fcb 100644
> > --- a/5_source_files/README.md
> > +++ b/5_source_files/README.md
> > @@ -33,12 +33,19 @@
> >
> >  ## 5.1 General Rules
> >
> > -### 5.1.1 Lines shall be 120 columns, or less
> > +### 5.1.1 Lines shall be 80 columns, or less
> >
> > -Preferably, limit line lengths to 80 columns or less. When
> this doesn't leave
> > -sufficient space for a good postfix style comment, extend the
> line to a total
> > -of 120 columns. Having some level of uniformity in the
> expected width of the
> > -source is useful for viewing and printing the code.
> > +Limit line lengths to 80 columns.
> > +
> > +Lines longer than 80 columns make it more difficult for the
> reader to find the
> > +beginning of the next line. They also tend to prevent users
> from displaying two
> > +source listings side-by-side on common display devices.
> > +
> > +When the 80 columns limit doesn't leave sufficient space for
> a postfix style
> > +comment, break the line into shorter segments at logical
> boundaries (for
> > +example, between the arguments of a function call, adhering
> to the spacing
> > +rules), or replace the postfix style comment with a
> standalone comment that
> > +precedes the statement.
> >
> >  ### 5.1.2 Do not use tab characters
> >
> > diff --git a/README.md b/README.md
> > index 8b9675b94937..8fad5a327b8c 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel
> Corporation. All rights reserved.
> >  | 2.1      | DRAFT for REFORMAT
> | 10/30/2015 |
> >  | 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
> |            |
> 
> Prompting the question as to whether the markdown for this table
> could
> reasonably be expressed in less that 173 columns? :)
> 
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Ard Biesheuvel]

On 11 August 2017 at 17:48, Laszlo Ersek <lersek@redhat.com> wrote:
> We currently say "stick with 80 if it's convenient, extend to 120
> otherwise".

It doesn't say that. It says you can make an exception for postfix
comments, which is not unreasonable imo.

This means most of the code in MdePkg/MdeModulePkg (afaik) already
violates the old coding styile, so what good is it going to do to
further restrict it?

> This is too lax; much new edk2 code ignores the 80 columns
> recommendation, resulting in source files that are hard to read for some
> contributors. Remove the 120 columns excuse and make 80 columns a
> requirement.
>
> 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/README.md | 17 ++++++++++++-----
>  README.md                |  1 +
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/5_source_files/README.md b/5_source_files/README.md
> index a93492db4f0f..546d44d94fcb 100644
> --- a/5_source_files/README.md
> +++ b/5_source_files/README.md
> @@ -33,12 +33,19 @@
>
>  ## 5.1 General Rules
>
> -### 5.1.1 Lines shall be 120 columns, or less
> +### 5.1.1 Lines shall be 80 columns, or less
>
> -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
> -sufficient space for a good postfix style comment, extend the line to a total
> -of 120 columns. Having some level of uniformity in the expected width of the
> -source is useful for viewing and printing the code.
> +Limit line lengths to 80 columns.
> +
> +Lines longer than 80 columns make it more difficult for the reader to find the
> +beginning of the next line. They also tend to prevent users from displaying two
> +source listings side-by-side on common display devices.
> +
> +When the 80 columns limit doesn't leave sufficient space for a postfix style
> +comment, break the line into shorter segments at logical boundaries (for
> +example, between the arguments of a function call, adhering to the spacing
> +rules), or replace the postfix style comment with a standalone comment that
> +precedes the statement.
>
>  ### 5.1.2 Do not use tab characters
>
> diff --git a/README.md b/README.md
> index 8b9675b94937..8fad5a327b8c 100644
> --- a/README.md
> +++ b/README.md
> @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
>  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
>  | 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                                                                                                                         |            |
> --
> 2.13.1.3.g8be5a757fa67
>
>

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Jordan Justen]

On 2017-08-11 15:52:44, Ard Biesheuvel wrote:
> On 11 August 2017 at 17:48, Laszlo Ersek <lersek@redhat.com> wrote:
> > We currently say "stick with 80 if it's convenient, extend to 120
> > otherwise".
> 
> It doesn't say that. It says you can make an exception for postfix
> comments, which is not unreasonable imo.
> 
> This means most of the code in MdePkg/MdeModulePkg (afaik) already
> violates the old coding styile, so what good is it going to do to
> further restrict it?

True, but I'm not sure it is widely known that the rule only applies
to postfix comments.

I guess we could add checking into 'PatchCheck.py' for line length,
and I guess it could handle the postfix exception as well. We might
also need to be more consistent with using PatchCheck.py... :)

The current 'rule' only carves out an exception for postfix comments,
and I guess I prefer to drop that. I'd rather see such (somewhat rare)
comments moved up if they are necessary.

Regarding violations, I decided to try to check this rule, and you are
correct in that it is often violated.

To find lines longer than 80 columns, I used:

$ git grep -P -e '^(?!\s{0,16}//)' --and \
              -e '^.{80}\s*[^\s]' --and \
              --not -e 'licen|accomp' -- *.c *.h | wc -l

'^(?!\s{0,16}//)': Skip non-postfix comment lines
'^.{80}\s*[^\s]': Lines with a non-space after 80 chars
'licen|accomp': avoid BSD license lines that are long

To find lines with postfix comments, I added:

'.*[^/].*//'

For MdePkg:
22244 long lines, and only 1522 long lines with a postfix comment.

For MdeModulePkg:
33248 long lines, and only 720 long lines with a postfix comment.

For OvmfPkg:
1116 long lines, and only 64 long lines with a postfix comment.

How about lines > 120 in length?

$ git grep -P '^.{120}\s*[^\s]' -- *.c *.h |wc -l

MdeModulePkg: 2017
MdePkg: 521
OvmfPkg: 45

-Jordan

> 
> > This is too lax; much new edk2 code ignores the 80 columns
> > recommendation, resulting in source files that are hard to read for some
> > contributors. Remove the 120 columns excuse and make 80 columns a
> > requirement.
> >
> > 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/README.md | 17 ++++++++++++-----
> >  README.md                |  1 +
> >  2 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/5_source_files/README.md b/5_source_files/README.md
> > index a93492db4f0f..546d44d94fcb 100644
> > --- a/5_source_files/README.md
> > +++ b/5_source_files/README.md
> > @@ -33,12 +33,19 @@
> >
> >  ## 5.1 General Rules
> >
> > -### 5.1.1 Lines shall be 120 columns, or less
> > +### 5.1.1 Lines shall be 80 columns, or less
> >
> > -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
> > -sufficient space for a good postfix style comment, extend the line to a total
> > -of 120 columns. Having some level of uniformity in the expected width of the
> > -source is useful for viewing and printing the code.
> > +Limit line lengths to 80 columns.
> > +
> > +Lines longer than 80 columns make it more difficult for the reader to find the
> > +beginning of the next line. They also tend to prevent users from displaying two
> > +source listings side-by-side on common display devices.
> > +
> > +When the 80 columns limit doesn't leave sufficient space for a postfix style
> > +comment, break the line into shorter segments at logical boundaries (for
> > +example, between the arguments of a function call, adhering to the spacing
> > +rules), or replace the postfix style comment with a standalone comment that
> > +precedes the statement.
> >
> >  ### 5.1.2 Do not use tab characters
> >
> > diff --git a/README.md b/README.md
> > index 8b9675b94937..8fad5a327b8c 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
> >  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
> >  | 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                                                                                                                         |            |
> > --
> > 2.13.1.3.g8be5a757fa67
> >
> >

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Leif Lindholm]

On Fri, Aug 11, 2017 at 11:52:44PM +0100, Ard Biesheuvel wrote:
> On 11 August 2017 at 17:48, Laszlo Ersek <lersek@redhat.com> wrote:
> > We currently say "stick with 80 if it's convenient, extend to 120
> > otherwise".
> 
> It doesn't say that. It says you can make an exception for postfix
> comments, which is not unreasonable imo.

Meh, I dislike postfix comments anyway.

> This means most of the code in MdePkg/MdeModulePkg (afaik) already
> violates the old coding styile, so what good is it going to do to
> further restrict it?

We can start applying it rigidly to new patches and have the codebase
improve over time.

Which will also work as a good canary for "hmm, no on has touched this
code in a very long time" :)

So for me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> > This is too lax; much new edk2 code ignores the 80 columns
> > recommendation, resulting in source files that are hard to read for some
> > contributors. Remove the 120 columns excuse and make 80 columns a
> > requirement.
> >
> > 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/README.md | 17 ++++++++++++-----
> >  README.md                |  1 +
> >  2 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/5_source_files/README.md b/5_source_files/README.md
> > index a93492db4f0f..546d44d94fcb 100644
> > --- a/5_source_files/README.md
> > +++ b/5_source_files/README.md
> > @@ -33,12 +33,19 @@
> >
> >  ## 5.1 General Rules
> >
> > -### 5.1.1 Lines shall be 120 columns, or less
> > +### 5.1.1 Lines shall be 80 columns, or less
> >
> > -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
> > -sufficient space for a good postfix style comment, extend the line to a total
> > -of 120 columns. Having some level of uniformity in the expected width of the
> > -source is useful for viewing and printing the code.
> > +Limit line lengths to 80 columns.
> > +
> > +Lines longer than 80 columns make it more difficult for the reader to find the
> > +beginning of the next line. They also tend to prevent users from displaying two
> > +source listings side-by-side on common display devices.
> > +
> > +When the 80 columns limit doesn't leave sufficient space for a postfix style
> > +comment, break the line into shorter segments at logical boundaries (for
> > +example, between the arguments of a function call, adhering to the spacing
> > +rules), or replace the postfix style comment with a standalone comment that
> > +precedes the statement.
> >
> >  ### 5.1.2 Do not use tab characters
> >
> > diff --git a/README.md b/README.md
> > index 8b9675b94937..8fad5a327b8c 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
> >  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
> >  | 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                                                                                                                         |            |
> > --
> > 2.13.1.3.g8be5a757fa67
> >
> >

Re: [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Laszlo Ersek]

On 08/12/17 00:52, Ard Biesheuvel wrote:
> On 11 August 2017 at 17:48, Laszlo Ersek <lersek@redhat.com> wrote:
>> We currently say "stick with 80 if it's convenient, extend to 120
>> otherwise".
> 
> It doesn't say that. It says you can make an exception for postfix
> comments, which is not unreasonable imo.

The way the section heading is worded (mentions 120 only), and how the
first paragraph starts ("preferably limit to 80"), are too easy to find
excuses out of.

> 
> This means most of the code in MdePkg/MdeModulePkg (afaik) already
> violates the old coding styile, so what good is it going to do to
> further restrict it?

It should affect new code.

(This is not the only example where we introduce a new style without
adapting the old code whole-sale. See for example EFI_D_* vs. DEBUG_*.)

Thanks
Laszlo

> 
>> This is too lax; much new edk2 code ignores the 80 columns
>> recommendation, resulting in source files that are hard to read for some
>> contributors. Remove the 120 columns excuse and make 80 columns a
>> requirement.
>>
>> 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/README.md | 17 ++++++++++++-----
>>  README.md                |  1 +
>>  2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/5_source_files/README.md b/5_source_files/README.md
>> index a93492db4f0f..546d44d94fcb 100644
>> --- a/5_source_files/README.md
>> +++ b/5_source_files/README.md
>> @@ -33,12 +33,19 @@
>>
>>  ## 5.1 General Rules
>>
>> -### 5.1.1 Lines shall be 120 columns, or less
>> +### 5.1.1 Lines shall be 80 columns, or less
>>
>> -Preferably, limit line lengths to 80 columns or less. When this doesn't leave
>> -sufficient space for a good postfix style comment, extend the line to a total
>> -of 120 columns. Having some level of uniformity in the expected width of the
>> -source is useful for viewing and printing the code.
>> +Limit line lengths to 80 columns.
>> +
>> +Lines longer than 80 columns make it more difficult for the reader to find the
>> +beginning of the next line. They also tend to prevent users from displaying two
>> +source listings side-by-side on common display devices.
>> +
>> +When the 80 columns limit doesn't leave sufficient space for a postfix style
>> +comment, break the line into shorter segments at logical boundaries (for
>> +example, between the arguments of a function call, adhering to the spacing
>> +rules), or replace the postfix style comment with a standalone comment that
>> +precedes the statement.
>>
>>  ### 5.1.2 Do not use tab characters
>>
>> diff --git a/README.md b/README.md
>> index 8b9675b94937..8fad5a327b8c 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -112,3 +112,4 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved.
>>  | 2.1      | DRAFT for REFORMAT                                                                                                                                | 10/30/2015 |
>>  | 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                                                                                                                         |            |
>> --
>> 2.13.1.3.g8be5a757fa67
>>
>>

Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Chang, Abner]

[-- Attachment #1: Type: text/plain, Size: 68 bytes --]

This change (1/2) will be merged to the CCS v2.3 release.

Abner

[-- Attachment #2: Type: text/html, Size: 76 bytes --]

Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Which change is that?  Does it match Uncrustify behavior?

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:25 PM
To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

This change (1/2) will be merged to the CCS v2.3 release.

Abner


[-- Attachment #2: Type: text/html, Size: 39234 bytes --]

Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

[Chang, Abner]

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

[AMD Official Use Only - General]

No, I don't think so. Need to update Uncrstify/PatchCheck tool if we have this patch merged to CCS.
Abner

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Sunday, November 13, 2022 9:59 AM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Which change is that?  Does it match Uncrustify behavior?

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:25 PM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General Rules: limit line lengths to 80 columns

This change (1/2) will be merged to the CCS v2.3 release.

Abner


[-- Attachment #2: Type: text/html, Size: 5326 bytes --]

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

[Laszlo Ersek]

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

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

[Jordan Justen]

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

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

[Kinney, Michael D]

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

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

[Jordan Justen]

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

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

[Andrew Fish]

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

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

[Leif Lindholm]

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

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

[Laszlo Ersek]

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

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

[Chang, Abner]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner

[-- Attachment #2: Type: text/html, Size: 556 bytes --]

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

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

Is this exactly what Uncrustify does now?

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:36 PM
To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner


[-- Attachment #2: Type: text/html, Size: 39667 bytes --]

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

[Chang, Abner]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

[AMD Official Use Only - General]

Uncrustify can fix the first argument that is not at the indent with two space. It also can fix the first argument that is not at the new line.
But it also makes each argument a new line if multiple args are condensed in one line. That is what we have to update Uncrustify if we have this patch merged to CCS.

+Michael Kubacki in loop.

Abner

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Sunday, November 13, 2022 9:58 AM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Is this exactly what Uncrustify does now?

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:36 PM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner


[-- Attachment #2: Type: text/html, Size: 6202 bytes --]

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

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 2884 bytes --]

We do not want another global format change because that make git blame difficult to use.

Are any clarifications required to describe the current Uncrustify behavior?  Or is the description correct?

If the current description matches Uncristify behavior, then I recommend we close this issue as will not fix.

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Sunday, November 13, 2022 12:45 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments


[AMD Official Use Only - General]

Uncrustify can fix the first argument that is not at the indent with two space. It also can fix the first argument that is not at the new line.
But it also makes each argument a new line if multiple args are condensed in one line. That is what we have to update Uncrustify if we have this patch merged to CCS.

+Michael Kubacki in loop.

Abner

From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Sunday, November 13, 2022 9:58 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Is this exactly what Uncrustify does now?

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:36 PM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner


[-- Attachment #2: Type: text/html, Size: 45176 bytes --]

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

[Chang, Abner]

[-- Attachment #1: Type: text/plain, Size: 4210 bytes --]

[AMD Official Use Only - General]

For this case, we don't have to take another global reformatting. These two formats can coexisting without the conflict.  We just allow the condense argus format in CSS. Also, update Uncrustify to not forcing each argument at its own line.
The current Uncrustify behavior seems to me match the CCS spec. But this patch was sent to allow the multiple argus at the same line, which was not proposed to fix the issue in current Uncrustify. You sure we just close this issue?
Abner


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney via groups.io
Sent: Monday, November 14, 2022 1:36 AM
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Laszlo Ersek <lersek@redhat.com>; Kubacki, Michael <michael.kubacki@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

We do not want another global format change because that make git blame difficult to use.

Are any clarifications required to describe the current Uncrustify behavior?  Or is the description correct?

If the current description matches Uncristify behavior, then I recommend we close this issue as will not fix.

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Sunday, November 13, 2022 12:45 AM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kubacki, Michael <michael.kubacki@microsoft.com<mailto:michael.kubacki@microsoft.com>>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments


[AMD Official Use Only - General]

Uncrustify can fix the first argument that is not at the indent with two space. It also can fix the first argument that is not at the new line.
But it also makes each argument a new line if multiple args are condensed in one line. That is what we have to update Uncrustify if we have this patch merged to CCS.

+Michael Kubacki in loop.

Abner

From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Sunday, November 13, 2022 9:58 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Is this exactly what Uncrustify does now?

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:36 PM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner


[-- Attachment #2: Type: text/html, Size: 10624 bytes --]

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

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 5220 bytes --]

I disagree that they can coexist.

If uncrustify is forcing 1 arg per line, then a developer that follows a CSS that allows multiple per line, the code change will be rejected by EDK II CI.

The CSS and Uncristify behavior need to be aligned.  If we want a CSS change that requires Uncristify changes, then they have to be coordinated.

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Sunday, November 13, 2022 5:10 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments


[AMD Official Use Only - General]

For this case, we don’t have to take another global reformatting. These two formats can coexisting without the conflict.  We just allow the condense argus format in CSS. Also, update Uncrustify to not forcing each argument at its own line.
The current Uncrustify behavior seems to me match the CCS spec. But this patch was sent to allow the multiple argus at the same line, which was not proposed to fix the issue in current Uncrustify. You sure we just close this issue?
Abner


From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Michael D Kinney via groups.io
Sent: Monday, November 14, 2022 1:36 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kubacki, Michael <michael.kubacki@microsoft.com<mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

We do not want another global format change because that make git blame difficult to use.

Are any clarifications required to describe the current Uncrustify behavior?  Or is the description correct?

If the current description matches Uncristify behavior, then I recommend we close this issue as will not fix.

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Sunday, November 13, 2022 12:45 AM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kubacki, Michael <michael.kubacki@microsoft.com<mailto:michael.kubacki@microsoft.com>>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments


[AMD Official Use Only - General]

Uncrustify can fix the first argument that is not at the indent with two space. It also can fix the first argument that is not at the new line.
But it also makes each argument a new line if multiple args are condensed in one line. That is what we have to update Uncrustify if we have this patch merged to CCS.

+Michael Kubacki in loop.

Abner

From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Sunday, November 13, 2022 9:58 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Is this exactly what Uncrustify does now?

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner via groups.io
Sent: Saturday, November 12, 2022 5:36 PM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

Hi all,
As we are going to release CCS 2.3, we would like to address some pending issues of CCS. For this, I think we can,
- Still keep the one line per argument style in CCS although the multi-arguments in the one line style can cover this. This avoids confusion from readers and questions about if they can do the one-line per argument style.
- If the arguments are in different lines, the first argument must be indented with two spaces from the start of the function name or the member function name.
How is this?

Abner


[-- Attachment #2: Type: text/html, Size: 49991 bytes --]

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

[Michael Kubacki]

I'm catching up on this thread so let me know if I miss something.

Uncrustify can perform conversions/enforcements like adjusting code to < 
80 columns.

The edk2 uncrustify configuration file is here and you will see that I 
commented column width enforcement:

https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19.

Uncrustify aside, column width is particularly difficult to adjust 
consistently. Both humans and tools have to make many (constant) 
situational decisions based on code structure.

However, it should be possible. I've taken the current uncrustify 
configuration file, made minimal changes to restrict code to 80 columns, 
and published the results based on the current edk2 code tree here:

https://github.com/makubacki/edk2/tree/uncrustify_80_column_change

You can see the I ran uncrustify 3 times to reach a mostly steady state. 
The issue after the second run is that uncrustify is confused about what 
to do with single macros that exceed 80 columns.

Examples:
1. 
https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704
2. 
https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031

There are other cases there as well.

Anyway, if those were reduced in length, I think we could reach a steady 
state. Some other minor tweaks might also be required.

Regarding function calls, I put together the following branch to 
demonstrate some examples of what is done now.

In summary, multiple arguments can be provided on a single line (with no 
width or argument count maximum) or multiple lines. If a single argument 
is multi-line, then all arguments must be on a unique line to follow the 
multi-line argument convention.

See the top two commits in this branch for examples:

https://github.com/makubacki/edk2/tree/func_arg_formatting_demo

I agree uncrustify and the spec be in sync.

Regards,
Michael

On 11/14/2022 12:07 PM, Michael D Kinney wrote:
> I disagree that they can coexist.
> 
> If uncrustify is forcing 1 arg per line, then a developer that follows a 
> CSS that allows multiple per line, the code change will be rejected by 
> EDK II CI.
> 
> The CSS and Uncristify behavior need to be aligned.If we want a CSS 
> change that requires Uncristify changes, then they have to be coordinated.
> 
> Mike
> 
> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Chang, 
> Abner via groups.io
> *Sent:* Sunday, November 13, 2022 5:10 PM
> *To:* devel@edk2.groups.io; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kubacki, 
> Michael <michael.kubacki@microsoft.com>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> [AMD Official Use Only - General]
> 
> For this case, we don’t have to take another global reformatting. These 
> two formats can coexisting without the conflict.  We just allow the 
> condense argus format in CSS. Also, update Uncrustify to not forcing 
> each argument at its own line.
> 
> The current Uncrustify behavior seems to me match the CCS spec. But this 
> patch was sent to allow the multiple argus at the same line, which was 
> not proposed to fix the issue in current Uncrustify. You sure we just 
> close this issue?
> 
> Abner
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Michael D Kinney via groups.io
> *Sent:* Monday, November 14, 2022 1:36 AM
> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael 
> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>; 
> Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> 	
> 
> *Caution:*This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> We do not want another global format change because that make git blame 
> difficult to use.
> 
> Are any clarifications required to describe the current Uncrustify 
> behavior?  Or is the description correct?
> 
> If the current description matches Uncristify behavior, then I recommend 
> we close this issue as will not fix.
> 
> Mike
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Chang, Abner via groups.io
> *Sent:* Sunday, November 13, 2022 12:45 AM
> *To:* Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io 
> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com 
> <mailto:lersek@redhat.com>>; Kubacki, Michael 
> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> [AMD Official Use Only - General]
> 
> Uncrustify can fix the first argument that is not at the indent with two 
> space. It also can fix the first argument that is not at the new line.
> 
> But it also makes each argument a new line if multiple args are 
> condensed in one line. That is what we have to update Uncrustify if we 
> have this patch merged to CCS.
> 
> +Michael Kubacki in loop.
> 
> Abner
> 
> *From:* Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>
> *Sent:* Sunday, November 13, 2022 9:58 AM
> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D 
> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> 	
> 
> *Caution:*This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> Is this exactly what Uncrustify does now?
> 
> Mike
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Chang, Abner via groups.io
> *Sent:* Saturday, November 12, 2022 5:36 PM
> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; 
> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> Hi all,
> As we are going to release CCS 2.3, we would like to address some 
> pending issues of CCS. For this, I think we can,
> - Still keep the one line per argument style in CCS although the 
> multi-arguments in the one line style can cover this. This avoids 
> confusion from readers and questions about if they can do the one-line 
> per argument style.
> - If the arguments are in different lines, the first argument must be 
> indented with two spaces from the start of the function name or the 
> member function name.
> How is this?
> 
> Abner
> 
> 

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

[Michael Kubacki]

Have these changes been discussed in a community forum? Do you think we 
could talk about it in the Tianocore Tools & CI Meeting today?

Thanks,
Michael

On 11/14/2022 12:37 PM, Michael Kubacki wrote:
> I'm catching up on this thread so let me know if I miss something.
> 
> Uncrustify can perform conversions/enforcements like adjusting code to < 
> 80 columns.
> 
> The edk2 uncrustify configuration file is here and you will see that I 
> commented column width enforcement:
> 
> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19. 
> 
> 
> Uncrustify aside, column width is particularly difficult to adjust 
> consistently. Both humans and tools have to make many (constant) 
> situational decisions based on code structure.
> 
> However, it should be possible. I've taken the current uncrustify 
> configuration file, made minimal changes to restrict code to 80 columns, 
> and published the results based on the current edk2 code tree here:
> 
> https://github.com/makubacki/edk2/tree/uncrustify_80_column_change
> 
> You can see the I ran uncrustify 3 times to reach a mostly steady state. 
> The issue after the second run is that uncrustify is confused about what 
> to do with single macros that exceed 80 columns.
> 
> Examples:
> 1. 
> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704 
> 
> 2. 
> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031 
> 
> 
> There are other cases there as well.
> 
> Anyway, if those were reduced in length, I think we could reach a steady 
> state. Some other minor tweaks might also be required.
> 
> Regarding function calls, I put together the following branch to 
> demonstrate some examples of what is done now.
> 
> In summary, multiple arguments can be provided on a single line (with no 
> width or argument count maximum) or multiple lines. If a single argument 
> is multi-line, then all arguments must be on a unique line to follow the 
> multi-line argument convention.
> 
> See the top two commits in this branch for examples:
> 
> https://github.com/makubacki/edk2/tree/func_arg_formatting_demo
> 
> I agree uncrustify and the spec be in sync.
> 
> Regards,
> Michael
> 
> On 11/14/2022 12:07 PM, Michael D Kinney wrote:
>> I disagree that they can coexist.
>>
>> If uncrustify is forcing 1 arg per line, then a developer that follows 
>> a CSS that allows multiple per line, the code change will be rejected 
>> by EDK II CI.
>>
>> The CSS and Uncristify behavior need to be aligned.If we want a CSS 
>> change that requires Uncristify changes, then they have to be 
>> coordinated.
>>
>> Mike
>>
>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of 
>> *Chang, Abner via groups.io
>> *Sent:* Sunday, November 13, 2022 5:10 PM
>> *To:* devel@edk2.groups.io; Kinney, Michael D 
>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; 
>> Kubacki, Michael <michael.kubacki@microsoft.com>
>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
>> arguments
>>
>> [AMD Official Use Only - General]
>>
>> For this case, we don’t have to take another global reformatting. 
>> These two formats can coexisting without the conflict.  We just allow 
>> the condense argus format in CSS. Also, update Uncrustify to not 
>> forcing each argument at its own line.
>>
>> The current Uncrustify behavior seems to me match the CCS spec. But 
>> this patch was sent to allow the multiple argus at the same line, 
>> which was not proposed to fix the issue in current Uncrustify. You 
>> sure we just close this issue?
>>
>> Abner
>>
>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
>> *Michael D Kinney via groups.io
>> *Sent:* Monday, November 14, 2022 1:36 AM
>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael 
>> <michael.kubacki@microsoft.com 
>> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D 
>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
>> arguments
>>
>>
>>
>> *Caution:*This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>> We do not want another global format change because that make git 
>> blame difficult to use.
>>
>> Are any clarifications required to describe the current Uncrustify 
>> behavior?  Or is the description correct?
>>
>> If the current description matches Uncristify behavior, then I 
>> recommend we close this issue as will not fix.
>>
>> Mike
>>
>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
>> *Chang, Abner via groups.io
>> *Sent:* Sunday, November 13, 2022 12:45 AM
>> *To:* Kinney, Michael D <michael.d.kinney@intel.com 
>> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io 
>> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com 
>> <mailto:lersek@redhat.com>>; Kubacki, Michael 
>> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
>> arguments
>>
>> [AMD Official Use Only - General]
>>
>> Uncrustify can fix the first argument that is not at the indent with 
>> two space. It also can fix the first argument that is not at the new 
>> line.
>>
>> But it also makes each argument a new line if multiple args are 
>> condensed in one line. That is what we have to update Uncrustify if we 
>> have this patch merged to CCS.
>>
>> +Michael Kubacki in loop.
>>
>> Abner
>>
>> *From:* Kinney, Michael D <michael.d.kinney@intel.com 
>> <mailto:michael.d.kinney@intel.com>>
>> *Sent:* Sunday, November 13, 2022 9:58 AM
>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D 
>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
>> arguments
>>
>>
>>
>> *Caution:*This message originated from an External Source. Use proper 
>> caution when opening attachments, clicking links, or responding.
>>
>> Is this exactly what Uncrustify does now?
>>
>> Mike
>>
>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
>> *Chang, Abner via groups.io
>> *Sent:* Saturday, November 12, 2022 5:36 PM
>> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; 
>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
>> arguments
>>
>> Hi all,
>> As we are going to release CCS 2.3, we would like to address some 
>> pending issues of CCS. For this, I think we can,
>> - Still keep the one line per argument style in CCS although the 
>> multi-arguments in the one line style can cover this. This avoids 
>> confusion from readers and questions about if they can do the one-line 
>> per argument style.
>> - If the arguments are in different lines, the first argument must be 
>> indented with two spaces from the start of the function name or the 
>> member function name.
>> How is this?
>>
>> Abner
>>
>>
> 
> 
> 
> 

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

[Michael D Kinney]

Hi Michael,

Yes.  They have been discussed.  There were patches and discussion on mailing
lists back in 2017.  Long before enabling uncrustify. I provided links to
these conversations in the following email about a week ago.

https://edk2.groups.io/g/devel/message/96038


Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Monday, November 14, 2022 10:05 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; abner.chang@amd.com; Laszlo Ersek <lersek@redhat.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> condensed arguments
> 
> Have these changes been discussed in a community forum? Do you think we
> could talk about it in the Tianocore Tools & CI Meeting today?
> 
> Thanks,
> Michael
> 
> On 11/14/2022 12:37 PM, Michael Kubacki wrote:
> > I'm catching up on this thread so let me know if I miss something.
> >
> > Uncrustify can perform conversions/enforcements like adjusting code to <
> > 80 columns.
> >
> > The edk2 uncrustify configuration file is here and you will see that I
> > commented column width enforcement:
> >
> > https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19.
> >
> >
> > Uncrustify aside, column width is particularly difficult to adjust
> > consistently. Both humans and tools have to make many (constant)
> > situational decisions based on code structure.
> >
> > However, it should be possible. I've taken the current uncrustify
> > configuration file, made minimal changes to restrict code to 80 columns,
> > and published the results based on the current edk2 code tree here:
> >
> > https://github.com/makubacki/edk2/tree/uncrustify_80_column_change
> >
> > You can see the I ran uncrustify 3 times to reach a mostly steady state.
> > The issue after the second run is that uncrustify is confused about what
> > to do with single macros that exceed 80 columns.
> >
> > Examples:
> > 1.
> > https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704
> >
> > 2.
> > https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031
> >
> >
> > There are other cases there as well.
> >
> > Anyway, if those were reduced in length, I think we could reach a steady
> > state. Some other minor tweaks might also be required.
> >
> > Regarding function calls, I put together the following branch to
> > demonstrate some examples of what is done now.
> >
> > In summary, multiple arguments can be provided on a single line (with no
> > width or argument count maximum) or multiple lines. If a single argument
> > is multi-line, then all arguments must be on a unique line to follow the
> > multi-line argument convention.
> >
> > See the top two commits in this branch for examples:
> >
> > https://github.com/makubacki/edk2/tree/func_arg_formatting_demo
> >
> > I agree uncrustify and the spec be in sync.
> >
> > Regards,
> > Michael
> >
> > On 11/14/2022 12:07 PM, Michael D Kinney wrote:
> >> I disagree that they can coexist.
> >>
> >> If uncrustify is forcing 1 arg per line, then a developer that follows
> >> a CSS that allows multiple per line, the code change will be rejected
> >> by EDK II CI.
> >>
> >> The CSS and Uncristify behavior need to be aligned.If we want a CSS
> >> change that requires Uncristify changes, then they have to be
> >> coordinated.
> >>
> >> Mike
> >>
> >> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of
> >> *Chang, Abner via groups.io
> >> *Sent:* Sunday, November 13, 2022 5:10 PM
> >> *To:* devel@edk2.groups.io; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >> Kubacki, Michael <michael.kubacki@microsoft.com>
> >> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >> arguments
> >>
> >> [AMD Official Use Only - General]
> >>
> >> For this case, we don’t have to take another global reformatting.
> >> These two formats can coexisting without the conflict.  We just allow
> >> the condense argus format in CSS. Also, update Uncrustify to not
> >> forcing each argument at its own line.
> >>
> >> The current Uncrustify behavior seems to me match the CCS spec. But
> >> this patch was sent to allow the multiple argus at the same line,
> >> which was not proposed to fix the issue in current Uncrustify. You
> >> sure we just close this issue?
> >>
> >> Abner
> >>
> >> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >> *Michael D Kinney via groups.io
> >> *Sent:* Monday, November 14, 2022 1:36 AM
> >> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
> >> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
> >> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael
> >> <michael.kubacki@microsoft.com
> >> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D
> >> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> >> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >> arguments
> >>
> >>
> >>
> >> *Caution:*This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >> We do not want another global format change because that make git
> >> blame difficult to use.
> >>
> >> Are any clarifications required to describe the current Uncrustify
> >> behavior?  Or is the description correct?
> >>
> >> If the current description matches Uncristify behavior, then I
> >> recommend we close this issue as will not fix.
> >>
> >> Mike
> >>
> >> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >> *Chang, Abner via groups.io
> >> *Sent:* Sunday, November 13, 2022 12:45 AM
> >> *To:* Kinney, Michael D <michael.d.kinney@intel.com
> >> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io
> >> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com
> >> <mailto:lersek@redhat.com>>; Kubacki, Michael
> >> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
> >> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >> arguments
> >>
> >> [AMD Official Use Only - General]
> >>
> >> Uncrustify can fix the first argument that is not at the indent with
> >> two space. It also can fix the first argument that is not at the new
> >> line.
> >>
> >> But it also makes each argument a new line if multiple args are
> >> condensed in one line. That is what we have to update Uncrustify if we
> >> have this patch merged to CCS.
> >>
> >> +Michael Kubacki in loop.
> >>
> >> Abner
> >>
> >> *From:* Kinney, Michael D <michael.d.kinney@intel.com
> >> <mailto:michael.d.kinney@intel.com>>
> >> *Sent:* Sunday, November 13, 2022 9:58 AM
> >> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
> >> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
> >> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D
> >> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> >> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >> arguments
> >>
> >>
> >>
> >> *Caution:*This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >> Is this exactly what Uncrustify does now?
> >>
> >> Mike
> >>
> >> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >> *Chang, Abner via groups.io
> >> *Sent:* Saturday, November 12, 2022 5:36 PM
> >> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>;
> >> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >> arguments
> >>
> >> Hi all,
> >> As we are going to release CCS 2.3, we would like to address some
> >> pending issues of CCS. For this, I think we can,
> >> - Still keep the one line per argument style in CCS although the
> >> multi-arguments in the one line style can cover this. This avoids
> >> confusion from readers and questions about if they can do the one-line
> >> per argument style.
> >> - If the arguments are in different lines, the first argument must be
> >> indented with two spaces from the start of the function name or the
> >> member function name.
> >> How is this?
> >>
> >> Abner
> >>
> >>
> >
> >
> >
> >
> 
> 
> 
> 

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

[Michael Kubacki]

I saw those, but they're from over 5 years ago and the community has 
changed since then.

This is an impactful change. It will impact every line of code written.

I'm not suggesting this go to the Tools & CI Meeting because of 
uncrustify. I'm not concerned with any impact to uncrustify.

This came to my attention because I was added to thread. I would like to 
suggest that we raise more awareness about this change in a meeting with 
members of the community to capture additional feedback.

Also, the logistics (uncrustify aside) about how to execute this change 
are unclear (as written in this thread and the BZ) and it would be 
easier/faster to discuss in a community call.

Is that possible?

Thanks,
Michael

On 11/14/2022 1:25 PM, Kinney, Michael D wrote:
> Hi Michael,
> 
> Yes.  They have been discussed.  There were patches and discussion on mailing
> lists back in 2017.  Long before enabling uncrustify. I provided links to
> these conversations in the following email about a week ago.
> 
> https://edk2.groups.io/g/devel/message/96038
> 
> 
> Mike
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
>> Sent: Monday, November 14, 2022 10:05 AM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; abner.chang@amd.com; Laszlo Ersek <lersek@redhat.com>;
>> Kubacki, Michael <michael.kubacki@microsoft.com>
>> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
>> condensed arguments
>>
>> Have these changes been discussed in a community forum? Do you think we
>> could talk about it in the Tianocore Tools & CI Meeting today?
>>
>> Thanks,
>> Michael
>>
>> On 11/14/2022 12:37 PM, Michael Kubacki wrote:
>>> I'm catching up on this thread so let me know if I miss something.
>>>
>>> Uncrustify can perform conversions/enforcements like adjusting code to <
>>> 80 columns.
>>>
>>> The edk2 uncrustify configuration file is here and you will see that I
>>> commented column width enforcement:
>>>
>>> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19.
>>>
>>>
>>> Uncrustify aside, column width is particularly difficult to adjust
>>> consistently. Both humans and tools have to make many (constant)
>>> situational decisions based on code structure.
>>>
>>> However, it should be possible. I've taken the current uncrustify
>>> configuration file, made minimal changes to restrict code to 80 columns,
>>> and published the results based on the current edk2 code tree here:
>>>
>>> https://github.com/makubacki/edk2/tree/uncrustify_80_column_change
>>>
>>> You can see the I ran uncrustify 3 times to reach a mostly steady state.
>>> The issue after the second run is that uncrustify is confused about what
>>> to do with single macros that exceed 80 columns.
>>>
>>> Examples:
>>> 1.
>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704
>>>
>>> 2.
>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031
>>>
>>>
>>> There are other cases there as well.
>>>
>>> Anyway, if those were reduced in length, I think we could reach a steady
>>> state. Some other minor tweaks might also be required.
>>>
>>> Regarding function calls, I put together the following branch to
>>> demonstrate some examples of what is done now.
>>>
>>> In summary, multiple arguments can be provided on a single line (with no
>>> width or argument count maximum) or multiple lines. If a single argument
>>> is multi-line, then all arguments must be on a unique line to follow the
>>> multi-line argument convention.
>>>
>>> See the top two commits in this branch for examples:
>>>
>>> https://github.com/makubacki/edk2/tree/func_arg_formatting_demo
>>>
>>> I agree uncrustify and the spec be in sync.
>>>
>>> Regards,
>>> Michael
>>>
>>> On 11/14/2022 12:07 PM, Michael D Kinney wrote:
>>>> I disagree that they can coexist.
>>>>
>>>> If uncrustify is forcing 1 arg per line, then a developer that follows
>>>> a CSS that allows multiple per line, the code change will be rejected
>>>> by EDK II CI.
>>>>
>>>> The CSS and Uncristify behavior need to be aligned.If we want a CSS
>>>> change that requires Uncristify changes, then they have to be
>>>> coordinated.
>>>>
>>>> Mike
>>>>
>>>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of
>>>> *Chang, Abner via groups.io
>>>> *Sent:* Sunday, November 13, 2022 5:10 PM
>>>> *To:* devel@edk2.groups.io; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>>>> Kubacki, Michael <michael.kubacki@microsoft.com>
>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>> arguments
>>>>
>>>> [AMD Official Use Only - General]
>>>>
>>>> For this case, we don’t have to take another global reformatting.
>>>> These two formats can coexisting without the conflict.  We just allow
>>>> the condense argus format in CSS. Also, update Uncrustify to not
>>>> forcing each argument at its own line.
>>>>
>>>> The current Uncrustify behavior seems to me match the CCS spec. But
>>>> this patch was sent to allow the multiple argus at the same line,
>>>> which was not proposed to fix the issue in current Uncrustify. You
>>>> sure we just close this issue?
>>>>
>>>> Abner
>>>>
>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>> *Michael D Kinney via groups.io
>>>> *Sent:* Monday, November 14, 2022 1:36 AM
>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
>>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
>>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael
>>>> <michael.kubacki@microsoft.com
>>>> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>> arguments
>>>>
>>>>
>>>>
>>>> *Caution:*This message originated from an External Source. Use proper
>>>> caution when opening attachments, clicking links, or responding.
>>>>
>>>> We do not want another global format change because that make git
>>>> blame difficult to use.
>>>>
>>>> Are any clarifications required to describe the current Uncrustify
>>>> behavior?  Or is the description correct?
>>>>
>>>> If the current description matches Uncristify behavior, then I
>>>> recommend we close this issue as will not fix.
>>>>
>>>> Mike
>>>>
>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>> *Chang, Abner via groups.io
>>>> *Sent:* Sunday, November 13, 2022 12:45 AM
>>>> *To:* Kinney, Michael D <michael.d.kinney@intel.com
>>>> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io
>>>> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com
>>>> <mailto:lersek@redhat.com>>; Kubacki, Michael
>>>> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>> arguments
>>>>
>>>> [AMD Official Use Only - General]
>>>>
>>>> Uncrustify can fix the first argument that is not at the indent with
>>>> two space. It also can fix the first argument that is not at the new
>>>> line.
>>>>
>>>> But it also makes each argument a new line if multiple args are
>>>> condensed in one line. That is what we have to update Uncrustify if we
>>>> have this patch merged to CCS.
>>>>
>>>> +Michael Kubacki in loop.
>>>>
>>>> Abner
>>>>
>>>> *From:* Kinney, Michael D <michael.d.kinney@intel.com
>>>> <mailto:michael.d.kinney@intel.com>>
>>>> *Sent:* Sunday, November 13, 2022 9:58 AM
>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
>>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
>>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>> arguments
>>>>
>>>>
>>>>
>>>> *Caution:*This message originated from an External Source. Use proper
>>>> caution when opening attachments, clicking links, or responding.
>>>>
>>>> Is this exactly what Uncrustify does now?
>>>>
>>>> Mike
>>>>
>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>> *Chang, Abner via groups.io
>>>> *Sent:* Saturday, November 12, 2022 5:36 PM
>>>> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>;
>>>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>> arguments
>>>>
>>>> Hi all,
>>>> As we are going to release CCS 2.3, we would like to address some
>>>> pending issues of CCS. For this, I think we can,
>>>> - Still keep the one line per argument style in CCS although the
>>>> multi-arguments in the one line style can cover this. This avoids
>>>> confusion from readers and questions about if they can do the one-line
>>>> per argument style.
>>>> - If the arguments are in different lines, the first argument must be
>>>> indented with two spaces from the start of the function name or the
>>>> member function name.
>>>> How is this?
>>>>
>>>> Abner
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>> 
>>
> 

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

[Michael D Kinney]

Hi Michael,

Yes. We can discuss in Tools/CI meeting later today.  Not just these specific issues
but also discuss process to coordinate changes to both CSS and Uncrustify config.

My recommendation is to close these older issues as "will not fix".  The review
of those topics 5 years were not completely resolved.  We have since added uncrustify
with CI checks.  If there are some elements of these previous topics that the 
community still thinks are valuable to consider, then those can be opened in a new
BZ and use the process that is established to coordinate the CSS and uncrustify
changes. 

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Monday, November 14, 2022 10:49 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; abner.chang@amd.com; Laszlo Ersek <lersek@redhat.com>;
> Kubacki, Michael <michael.kubacki@microsoft.com>
> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> condensed arguments
> 
> I saw those, but they're from over 5 years ago and the community has
> changed since then.
> 
> This is an impactful change. It will impact every line of code written.
> 
> I'm not suggesting this go to the Tools & CI Meeting because of
> uncrustify. I'm not concerned with any impact to uncrustify.
> 
> This came to my attention because I was added to thread. I would like to
> suggest that we raise more awareness about this change in a meeting with
> members of the community to capture additional feedback.
> 
> Also, the logistics (uncrustify aside) about how to execute this change
> are unclear (as written in this thread and the BZ) and it would be
> easier/faster to discuss in a community call.
> 
> Is that possible?
> 
> Thanks,
> Michael
> 
> On 11/14/2022 1:25 PM, Kinney, Michael D wrote:
> > Hi Michael,
> >
> > Yes.  They have been discussed.  There were patches and discussion on mailing
> > lists back in 2017.  Long before enabling uncrustify. I provided links to
> > these conversations in the following email about a week ago.
> >
> > https://edk2.groups.io/g/devel/message/96038
> >
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> >> Sent: Monday, November 14, 2022 10:05 AM
> >> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; abner.chang@amd.com; Laszlo Ersek
> <lersek@redhat.com>;
> >> Kubacki, Michael <michael.kubacki@microsoft.com>
> >> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls:
> allow
> >> condensed arguments
> >>
> >> Have these changes been discussed in a community forum? Do you think we
> >> could talk about it in the Tianocore Tools & CI Meeting today?
> >>
> >> Thanks,
> >> Michael
> >>
> >> On 11/14/2022 12:37 PM, Michael Kubacki wrote:
> >>> I'm catching up on this thread so let me know if I miss something.
> >>>
> >>> Uncrustify can perform conversions/enforcements like adjusting code to <
> >>> 80 columns.
> >>>
> >>> The edk2 uncrustify configuration file is here and you will see that I
> >>> commented column width enforcement:
> >>>
> >>> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19.
> >>>
> >>>
> >>> Uncrustify aside, column width is particularly difficult to adjust
> >>> consistently. Both humans and tools have to make many (constant)
> >>> situational decisions based on code structure.
> >>>
> >>> However, it should be possible. I've taken the current uncrustify
> >>> configuration file, made minimal changes to restrict code to 80 columns,
> >>> and published the results based on the current edk2 code tree here:
> >>>
> >>> https://github.com/makubacki/edk2/tree/uncrustify_80_column_change
> >>>
> >>> You can see the I ran uncrustify 3 times to reach a mostly steady state.
> >>> The issue after the second run is that uncrustify is confused about what
> >>> to do with single macros that exceed 80 columns.
> >>>
> >>> Examples:
> >>> 1.
> >>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704
> >>>
> >>> 2.
> >>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-
> #L1031
> >>>
> >>>
> >>> There are other cases there as well.
> >>>
> >>> Anyway, if those were reduced in length, I think we could reach a steady
> >>> state. Some other minor tweaks might also be required.
> >>>
> >>> Regarding function calls, I put together the following branch to
> >>> demonstrate some examples of what is done now.
> >>>
> >>> In summary, multiple arguments can be provided on a single line (with no
> >>> width or argument count maximum) or multiple lines. If a single argument
> >>> is multi-line, then all arguments must be on a unique line to follow the
> >>> multi-line argument convention.
> >>>
> >>> See the top two commits in this branch for examples:
> >>>
> >>> https://github.com/makubacki/edk2/tree/func_arg_formatting_demo
> >>>
> >>> I agree uncrustify and the spec be in sync.
> >>>
> >>> Regards,
> >>> Michael
> >>>
> >>> On 11/14/2022 12:07 PM, Michael D Kinney wrote:
> >>>> I disagree that they can coexist.
> >>>>
> >>>> If uncrustify is forcing 1 arg per line, then a developer that follows
> >>>> a CSS that allows multiple per line, the code change will be rejected
> >>>> by EDK II CI.
> >>>>
> >>>> The CSS and Uncristify behavior need to be aligned.If we want a CSS
> >>>> change that requires Uncristify changes, then they have to be
> >>>> coordinated.
> >>>>
> >>>> Mike
> >>>>
> >>>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of
> >>>> *Chang, Abner via groups.io
> >>>> *Sent:* Sunday, November 13, 2022 5:10 PM
> >>>> *To:* devel@edk2.groups.io; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >>>> Kubacki, Michael <michael.kubacki@microsoft.com>
> >>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >>>> arguments
> >>>>
> >>>> [AMD Official Use Only - General]
> >>>>
> >>>> For this case, we don’t have to take another global reformatting.
> >>>> These two formats can coexisting without the conflict.  We just allow
> >>>> the condense argus format in CSS. Also, update Uncrustify to not
> >>>> forcing each argument at its own line.
> >>>>
> >>>> The current Uncrustify behavior seems to me match the CCS spec. But
> >>>> this patch was sent to allow the multiple argus at the same line,
> >>>> which was not proposed to fix the issue in current Uncrustify. You
> >>>> sure we just close this issue?
> >>>>
> >>>> Abner
> >>>>
> >>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >>>> *Michael D Kinney via groups.io
> >>>> *Sent:* Monday, November 14, 2022 1:36 AM
> >>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
> >>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
> >>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael
> >>>> <michael.kubacki@microsoft.com
> >>>> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> >>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >>>> arguments
> >>>>
> >>>>
> >>>>
> >>>> *Caution:*This message originated from an External Source. Use proper
> >>>> caution when opening attachments, clicking links, or responding.
> >>>>
> >>>> We do not want another global format change because that make git
> >>>> blame difficult to use.
> >>>>
> >>>> Are any clarifications required to describe the current Uncrustify
> >>>> behavior?  Or is the description correct?
> >>>>
> >>>> If the current description matches Uncristify behavior, then I
> >>>> recommend we close this issue as will not fix.
> >>>>
> >>>> Mike
> >>>>
> >>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >>>> *Chang, Abner via groups.io
> >>>> *Sent:* Sunday, November 13, 2022 12:45 AM
> >>>> *To:* Kinney, Michael D <michael.d.kinney@intel.com
> >>>> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io
> >>>> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com
> >>>> <mailto:lersek@redhat.com>>; Kubacki, Michael
> >>>> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
> >>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >>>> arguments
> >>>>
> >>>> [AMD Official Use Only - General]
> >>>>
> >>>> Uncrustify can fix the first argument that is not at the indent with
> >>>> two space. It also can fix the first argument that is not at the new
> >>>> line.
> >>>>
> >>>> But it also makes each argument a new line if multiple args are
> >>>> condensed in one line. That is what we have to update Uncrustify if we
> >>>> have this patch merged to CCS.
> >>>>
> >>>> +Michael Kubacki in loop.
> >>>>
> >>>> Abner
> >>>>
> >>>> *From:* Kinney, Michael D <michael.d.kinney@intel.com
> >>>> <mailto:michael.d.kinney@intel.com>>
> >>>> *Sent:* Sunday, November 13, 2022 9:58 AM
> >>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner
> >>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
> >>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> >>>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >>>> arguments
> >>>>
> >>>>
> >>>>
> >>>> *Caution:*This message originated from an External Source. Use proper
> >>>> caution when opening attachments, clicking links, or responding.
> >>>>
> >>>> Is this exactly what Uncrustify does now?
> >>>>
> >>>> Mike
> >>>>
> >>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
> >>>> *Chang, Abner via groups.io
> >>>> *Sent:* Saturday, November 12, 2022 5:36 PM
> >>>> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>;
> >>>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
> >>>> arguments
> >>>>
> >>>> Hi all,
> >>>> As we are going to release CCS 2.3, we would like to address some
> >>>> pending issues of CCS. For this, I think we can,
> >>>> - Still keep the one line per argument style in CCS although the
> >>>> multi-arguments in the one line style can cover this. This avoids
> >>>> confusion from readers and questions about if they can do the one-line
> >>>> per argument style.
> >>>> - If the arguments are in different lines, the first argument must be
> >>>> indented with two spaces from the start of the function name or the
> >>>> member function name.
> >>>> How is this?
> >>>>
> >>>> Abner
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >>
> >
> 
> 
> 
> 

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

[Sean]

If you look through the bugs you will see there is some debate. 
Personally, I think there is never a single right answer on style and 
there will always be differing opinions.  The underlying goal that most 
agree on is providing consistency for a project and to make it easy for 
someone to contribute code that aligns with that consistent standard.   
Since 2017, a lot has changed but the biggest thing is we have a 
relatively easy, well documented, industry standard tool that supports 
style/formatting of edk2. These tools can be run locally by those 
contributing and are run by the CI system on pull request to enforce 
this consistency.  We have already reformatted the entire code base of 
edk2 to align with that and although i am not opposed to auto 
reformatting in the future for a great reason I don't see anything is 
these proposed changes that would justify that.


Thanks

Sean



On 11/14/2022 10:49 AM, Michael Kubacki wrote:
> I saw those, but they're from over 5 years ago and the community has 
> changed since then.
>
> This is an impactful change. It will impact every line of code written.
>
> I'm not suggesting this go to the Tools & CI Meeting because of 
> uncrustify. I'm not concerned with any impact to uncrustify.
>
> This came to my attention because I was added to thread. I would like 
> to suggest that we raise more awareness about this change in a meeting 
> with members of the community to capture additional feedback.
>
> Also, the logistics (uncrustify aside) about how to execute this 
> change are unclear (as written in this thread and the BZ) and it would 
> be easier/faster to discuss in a community call.
>
> Is that possible?
>
> Thanks,
> Michael
>
> On 11/14/2022 1:25 PM, Kinney, Michael D wrote:
>> Hi Michael,
>>
>> Yes.  They have been discussed.  There were patches and discussion on 
>> mailing
>> lists back in 2017.  Long before enabling uncrustify. I provided 
>> links to
>> these conversations in the following email about a week ago.
>>
>> https://edk2.groups.io/g/devel/message/96038
>>
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
>>> Michael Kubacki
>>> Sent: Monday, November 14, 2022 10:05 AM
>>> To: devel@edk2.groups.io; Kinney, Michael D 
>>> <michael.d.kinney@intel.com>; abner.chang@amd.com; Laszlo Ersek 
>>> <lersek@redhat.com>;
>>> Kubacki, Michael <michael.kubacki@microsoft.com>
>>> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow
>>> condensed arguments
>>>
>>> Have these changes been discussed in a community forum? Do you think we
>>> could talk about it in the Tianocore Tools & CI Meeting today?
>>>
>>> Thanks,
>>> Michael
>>>
>>> On 11/14/2022 12:37 PM, Michael Kubacki wrote:
>>>> I'm catching up on this thread so let me know if I miss something.
>>>>
>>>> Uncrustify can perform conversions/enforcements like adjusting code 
>>>> to <
>>>> 80 columns.
>>>>
>>>> The edk2 uncrustify configuration file is here and you will see that I
>>>> commented column width enforcement:
>>>>
>>>> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19. 
>>>>
>>>>
>>>>
>>>> Uncrustify aside, column width is particularly difficult to adjust
>>>> consistently. Both humans and tools have to make many (constant)
>>>> situational decisions based on code structure.
>>>>
>>>> However, it should be possible. I've taken the current uncrustify
>>>> configuration file, made minimal changes to restrict code to 80 
>>>> columns,
>>>> and published the results based on the current edk2 code tree here:
>>>>
>>>> https://github.com/makubacki/edk2/tree/uncrustify_80_column_change
>>>>
>>>> You can see the I ran uncrustify 3 times to reach a mostly steady 
>>>> state.
>>>> The issue after the second run is that uncrustify is confused about 
>>>> what
>>>> to do with single macros that exceed 80 columns.
>>>>
>>>> Examples:
>>>> 1.
>>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704 
>>>>
>>>>
>>>> 2.
>>>> https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031 
>>>>
>>>>
>>>>
>>>> There are other cases there as well.
>>>>
>>>> Anyway, if those were reduced in length, I think we could reach a 
>>>> steady
>>>> state. Some other minor tweaks might also be required.
>>>>
>>>> Regarding function calls, I put together the following branch to
>>>> demonstrate some examples of what is done now.
>>>>
>>>> In summary, multiple arguments can be provided on a single line 
>>>> (with no
>>>> width or argument count maximum) or multiple lines. If a single 
>>>> argument
>>>> is multi-line, then all arguments must be on a unique line to 
>>>> follow the
>>>> multi-line argument convention.
>>>>
>>>> See the top two commits in this branch for examples:
>>>>
>>>> https://github.com/makubacki/edk2/tree/func_arg_formatting_demo
>>>>
>>>> I agree uncrustify and the spec be in sync.
>>>>
>>>> Regards,
>>>> Michael
>>>>
>>>> On 11/14/2022 12:07 PM, Michael D Kinney wrote:
>>>>> I disagree that they can coexist.
>>>>>
>>>>> If uncrustify is forcing 1 arg per line, then a developer that 
>>>>> follows
>>>>> a CSS that allows multiple per line, the code change will be rejected
>>>>> by EDK II CI.
>>>>>
>>>>> The CSS and Uncristify behavior need to be aligned.If we want a CSS
>>>>> change that requires Uncristify changes, then they have to be
>>>>> coordinated.
>>>>>
>>>>> Mike
>>>>>
>>>>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of
>>>>> *Chang, Abner via groups.io
>>>>> *Sent:* Sunday, November 13, 2022 5:10 PM
>>>>> *To:* devel@edk2.groups.io; Kinney, Michael D
>>>>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>>>>> Kubacki, Michael <michael.kubacki@microsoft.com>
>>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>>> arguments
>>>>>
>>>>> [AMD Official Use Only - General]
>>>>>
>>>>> For this case, we don’t have to take another global reformatting.
>>>>> These two formats can coexisting without the conflict.  We just allow
>>>>> the condense argus format in CSS. Also, update Uncrustify to not
>>>>> forcing each argument at its own line.
>>>>>
>>>>> The current Uncrustify behavior seems to me match the CCS spec. But
>>>>> this patch was sent to allow the multiple argus at the same line,
>>>>> which was not proposed to fix the issue in current Uncrustify. You
>>>>> sure we just close this issue?
>>>>>
>>>>> Abner
>>>>>
>>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>>> *Michael D Kinney via groups.io
>>>>> *Sent:* Monday, November 14, 2022 1:36 AM
>>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, 
>>>>> Abner
>>>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
>>>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael
>>>>> <michael.kubacki@microsoft.com
>>>>> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D
>>>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>>> arguments
>>>>>
>>>>>
>>>>>
>>>>> *Caution:*This message originated from an External Source. Use proper
>>>>> caution when opening attachments, clicking links, or responding.
>>>>>
>>>>> We do not want another global format change because that make git
>>>>> blame difficult to use.
>>>>>
>>>>> Are any clarifications required to describe the current Uncrustify
>>>>> behavior?  Or is the description correct?
>>>>>
>>>>> If the current description matches Uncristify behavior, then I
>>>>> recommend we close this issue as will not fix.
>>>>>
>>>>> Mike
>>>>>
>>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>>> *Chang, Abner via groups.io
>>>>> *Sent:* Sunday, November 13, 2022 12:45 AM
>>>>> *To:* Kinney, Michael D <michael.d.kinney@intel.com
>>>>> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io
>>>>> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com
>>>>> <mailto:lersek@redhat.com>>; Kubacki, Michael
>>>>> <michael.kubacki@microsoft.com 
>>>>> <mailto:michael.kubacki@microsoft.com>>
>>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>>> arguments
>>>>>
>>>>> [AMD Official Use Only - General]
>>>>>
>>>>> Uncrustify can fix the first argument that is not at the indent with
>>>>> two space. It also can fix the first argument that is not at the new
>>>>> line.
>>>>>
>>>>> But it also makes each argument a new line if multiple args are
>>>>> condensed in one line. That is what we have to update Uncrustify 
>>>>> if we
>>>>> have this patch merged to CCS.
>>>>>
>>>>> +Michael Kubacki in loop.
>>>>>
>>>>> Abner
>>>>>
>>>>> *From:* Kinney, Michael D <michael.d.kinney@intel.com
>>>>> <mailto:michael.d.kinney@intel.com>>
>>>>> *Sent:* Sunday, November 13, 2022 9:58 AM
>>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, 
>>>>> Abner
>>>>> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek
>>>>> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D
>>>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>>>>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>>> arguments
>>>>>
>>>>>
>>>>>
>>>>> *Caution:*This message originated from an External Source. Use proper
>>>>> caution when opening attachments, clicking links, or responding.
>>>>>
>>>>> Is this exactly what Uncrustify does now?
>>>>>
>>>>> Mike
>>>>>
>>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of
>>>>> *Chang, Abner via groups.io
>>>>> *Sent:* Saturday, November 12, 2022 5:36 PM
>>>>> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>;
>>>>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
>>>>> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed
>>>>> arguments
>>>>>
>>>>> Hi all,
>>>>> As we are going to release CCS 2.3, we would like to address some
>>>>> pending issues of CCS. For this, I think we can,
>>>>> - Still keep the one line per argument style in CCS although the
>>>>> multi-arguments in the one line style can cover this. This avoids
>>>>> confusion from readers and questions about if they can do the 
>>>>> one-line
>>>>> per argument style.
>>>>> - If the arguments are in different lines, the first argument must be
>>>>> indented with two spaces from the start of the function name or the
>>>>> member function name.
>>>>> How is this?
>>>>>
>>>>> Abner
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>
>
> 
>
>

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

[Chang, Abner]

[AMD Official Use Only - General]

Seem we have the consensus to change this BZ to WONTFIX, reopen it if necessary after the further discussions. The "code > 80 columns" and "the multiple argus at one line" are not considered to be part of CCS v2.3 release also.

Abner

> -----Original Message-----
> From: Sean Brogan <spbrogan@outlook.com>
> Sent: Tuesday, November 15, 2022 3:09 AM
> To: devel@edk2.groups.io; mikuback@linux.microsoft.com; Kinney, Michael
> D <michael.d.kinney@intel.com>; Chang, Abner <Abner.Chang@amd.com>;
> Laszlo Ersek <lersek@redhat.com>; Kubacki, Michael
> <michael.kubacki@microsoft.com>
> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2]
> Source Files / Spacing / Multi-line func. calls: allow condensed arguments
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> If you look through the bugs you will see there is some debate.
> Personally, I think there is never a single right answer on style and there will
> always be differing opinions.  The underlying goal that most agree on is
> providing consistency for a project and to make it easy for someone to
> contribute code that aligns with that consistent standard.
> Since 2017, a lot has changed but the biggest thing is we have a relatively
> easy, well documented, industry standard tool that supports
> style/formatting of edk2. These tools can be run locally by those contributing
> and are run by the CI system on pull request to enforce this consistency.  We
> have already reformatted the entire code base of
> edk2 to align with that and although i am not opposed to auto reformatting in
> the future for a great reason I don't see anything is these proposed changes
> that would justify that.
> 
> 
> Thanks
> 
> Sean
> 
> 
> 
> On 11/14/2022 10:49 AM, Michael Kubacki wrote:
> > I saw those, but they're from over 5 years ago and the community has
> > changed since then.
> >
> > This is an impactful change. It will impact every line of code written.
> >
> > I'm not suggesting this go to the Tools & CI Meeting because of
> > uncrustify. I'm not concerned with any impact to uncrustify.
> >
> > This came to my attention because I was added to thread. I would like
> > to suggest that we raise more awareness about this change in a meeting
> > with members of the community to capture additional feedback.
> >
> > Also, the logistics (uncrustify aside) about how to execute this
> > change are unclear (as written in this thread and the BZ) and it would
> > be easier/faster to discuss in a community call.
> >
> > Is that possible?
> >
> > Thanks,
> > Michael
> >
> > On 11/14/2022 1:25 PM, Kinney, Michael D wrote:
> >> Hi Michael,
> >>
> >> Yes.  They have been discussed.  There were patches and discussion on
> >> mailing lists back in 2017.  Long before enabling uncrustify. I
> >> provided links to these conversations in the following email about a
> >> week ago.
> >>
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
> >>
> 2.groups.io%2Fg%2Fdevel%2Fmessage%2F96038&amp;data=05%7C01%7Cab
> ner.ch
> >>
> ang%40amd.com%7C1f9c0bc3c6c747f1297308dac673afe9%7C3dd8961fe4884
> e608e
> >>
> 11a82d994e183d%7C0%7C0%7C638040497500028713%7CUnknown%7CTWFp
> bGZsb3d8e
> >>
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C3
> >>
> 000%7C%7C%7C&amp;sdata=TEqQG9CS8IgF4x33PsIufmwLfM6Tpj%2Be8JQx
> 9ZxeqHo%
> >> 3D&amp;reserved=0
> >>
> >>
> >> Mike
> >>
> >>> -----Original Message-----
> >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >>> Michael Kubacki
> >>> Sent: Monday, November 14, 2022 10:05 AM
> >>> To: devel@edk2.groups.io; Kinney, Michael D
> >>> <michael.d.kinney@intel.com>; abner.chang@amd.com; Laszlo Ersek
> >>> <lersek@redhat.com>; Kubacki, Michael
> >>> <michael.kubacki@microsoft.com>
> >>> Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH
> >>> 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>> condensed arguments
> >>>
> >>> Have these changes been discussed in a community forum? Do you think
> >>> we could talk about it in the Tianocore Tools & CI Meeting today?
> >>>
> >>> Thanks,
> >>> Michael
> >>>
> >>> On 11/14/2022 12:37 PM, Michael Kubacki wrote:
> >>>> I'm catching up on this thread so let me know if I miss something.
> >>>>
> >>>> Uncrustify can perform conversions/enforcements like adjusting code
> >>>> to <
> >>>> 80 columns.
> >>>>
> >>>> The edk2 uncrustify configuration file is here and you will see
> >>>> that I commented column width enforcement:
> >>>>
> >>>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2F.pytool%2FPlugin%2FU
> ncrustifyCheck%2Funcrustify.cfg%23L19&amp;data=05%7C01%7Cabner.chan
> g%40amd.com%7C1f9c0bc3c6c747f1297308dac673afe9%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C638040497500028713%7CUnknown%7CT
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLC
> JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6CpnXs63WTGicnIznfXtG
> MZHWjgq24Sz3yC8uWzEwQA%3D&amp;reserved=0.
> >>>>
> >>>>
> >>>>
> >>>> Uncrustify aside, column width is particularly difficult to adjust
> >>>> consistently. Both humans and tools have to make many (constant)
> >>>> situational decisions based on code structure.
> >>>>
> >>>> However, it should be possible. I've taken the current uncrustify
> >>>> configuration file, made minimal changes to restrict code to 80
> >>>> columns, and published the results based on the current edk2 code
> >>>> tree here:
> >>>>
> >>>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >>>>
> ithub.com%2Fmakubacki%2Fedk2%2Ftree%2Funcrustify_80_column_chang
> e&a
> >>>>
> mp;data=05%7C01%7Cabner.chang%40amd.com%7C1f9c0bc3c6c747f1297308
> dac
> >>>>
> 673afe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6380404975
> 0002
> >>>>
> 8713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLC
> >>>>
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=7OJjrlwy
> Eq%2
> >>>> FPdbYEZFFsVvHsYs0ir0cVuLMWy4wxLEM%3D&amp;reserved=0
> >>>>
> >>>> You can see the I ran uncrustify 3 times to reach a mostly steady
> >>>> state.
> >>>> The issue after the second run is that uncrustify is confused about
> >>>> what to do with single macros that exceed 80 columns.
> >>>>
> >>>> Examples:
> >>>> 1.
> >>>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >>>>
> ithub.com%2Fmakubacki%2Fedk2%2Fblob%2Funcrustify_80_column_chang
> e%2
> >>>> FMdePkg%2FInclude%2FIndustryStandard%2FAcpi30.h%23L702-
> %23L704&amp;
> >>>>
> data=05%7C01%7Cabner.chang%40amd.com%7C1f9c0bc3c6c747f1297308dac
> 673
> >>>>
> afe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6380404975000
> 2871
> >>>>
> 3%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzI
> iLCJBT
> >>>>
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=1nCH5GoBl
> LU9Lb0
> >>>> r%2FVzuEaPg2mXTS3JGpLi9UBwRTYk%3D&amp;reserved=0
> >>>>
> >>>>
> >>>> 2.
> >>>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >>>>
> ithub.com%2Fmakubacki%2Fedk2%2Fblob%2Funcrustify_80_column_chang
> e%2
> >>>>
> FMdePkg%2FInclude%2FIndustryStandard%2FIpmiNetFnApp.h%23L1023-
> %23L1
> >>>>
> 031&amp;data=05%7C01%7Cabner.chang%40amd.com%7C1f9c0bc3c6c747f1
> 2973
> >>>>
> 08dac673afe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63804
> 0497
> >>>>
> 500028713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luM
> >>>>
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Sw
> %2BVP
> >>>> dtwl8ydIWSq3B80hDz2luABjdXsP91O2WcJ0aA%3D&amp;reserved=0
> >>>>
> >>>>
> >>>>
> >>>> There are other cases there as well.
> >>>>
> >>>> Anyway, if those were reduced in length, I think we could reach a
> >>>> steady state. Some other minor tweaks might also be required.
> >>>>
> >>>> Regarding function calls, I put together the following branch to
> >>>> demonstrate some examples of what is done now.
> >>>>
> >>>> In summary, multiple arguments can be provided on a single line
> >>>> (with no width or argument count maximum) or multiple lines. If a
> >>>> single argument is multi-line, then all arguments must be on a
> >>>> unique line to follow the multi-line argument convention.
> >>>>
> >>>> See the top two commits in this branch for examples:
> >>>>
> >>>>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> >>>>
> ithub.com%2Fmakubacki%2Fedk2%2Ftree%2Ffunc_arg_formatting_demo&
> amp;
> >>>>
> data=05%7C01%7Cabner.chang%40amd.com%7C1f9c0bc3c6c747f1297308dac
> 673
> >>>>
> afe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6380404975000
> 2871
> >>>>
> 3%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzI
> iLCJBT
> >>>>
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Y2MTi5v5Fl
> PuEdx
> >>>> 5Lmg8%2Fm15TK7rNwlYapK%2FBExHC6o%3D&amp;reserved=0
> >>>>
> >>>> I agree uncrustify and the spec be in sync.
> >>>>
> >>>> Regards,
> >>>> Michael
> >>>>
> >>>> On 11/14/2022 12:07 PM, Michael D Kinney wrote:
> >>>>> I disagree that they can coexist.
> >>>>>
> >>>>> If uncrustify is forcing 1 arg per line, then a developer that
> >>>>> follows a CSS that allows multiple per line, the code change will
> >>>>> be rejected by EDK II CI.
> >>>>>
> >>>>> The CSS and Uncristify behavior need to be aligned.If we want a
> >>>>> CSS change that requires Uncristify changes, then they have to be
> >>>>> coordinated.
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf
> Of
> >>>>> *Chang, Abner via groups.io
> >>>>> *Sent:* Sunday, November 13, 2022 5:10 PM
> >>>>> *To:* devel@edk2.groups.io; Kinney, Michael D
> >>>>> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >>>>> Kubacki, Michael <michael.kubacki@microsoft.com>
> >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification
> >>>>> PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>>>> condensed arguments
> >>>>>
> >>>>> [AMD Official Use Only - General]
> >>>>>
> >>>>> For this case, we don't have to take another global reformatting.
> >>>>> These two formats can coexisting without the conflict.  We just
> >>>>> allow the condense argus format in CSS. Also, update Uncrustify to
> >>>>> not forcing each argument at its own line.
> >>>>>
> >>>>> The current Uncrustify behavior seems to me match the CCS spec.
> >>>>> But this patch was sent to allow the multiple argus at the same
> >>>>> line, which was not proposed to fix the issue in current
> >>>>> Uncrustify. You sure we just close this issue?
> >>>>>
> >>>>> Abner
> >>>>>
> >>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf
> Of
> >>>>> *Michael D Kinney via groups.io
> >>>>> *Sent:* Monday, November 14, 2022 1:36 AM
> >>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang,
> >>>>> Abner <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>;
> Laszlo
> >>>>> Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki,
> >>>>> Michael <michael.kubacki@microsoft.com
> >>>>> <mailto:michael.kubacki@microsoft.com>>; Kinney, Michael D
> >>>>> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification
> >>>>> PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>>>> condensed arguments
> >>>>>
> >>>>>
> >>>>>
> >>>>> *Caution:*This message originated from an External Source. Use
> >>>>> proper caution when opening attachments, clicking links, or
> responding.
> >>>>>
> >>>>> We do not want another global format change because that make git
> >>>>> blame difficult to use.
> >>>>>
> >>>>> Are any clarifications required to describe the current Uncrustify
> >>>>> behavior?  Or is the description correct?
> >>>>>
> >>>>> If the current description matches Uncristify behavior, then I
> >>>>> recommend we close this issue as will not fix.
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf
> Of
> >>>>> *Chang, Abner via groups.io
> >>>>> *Sent:* Sunday, November 13, 2022 12:45 AM
> >>>>> *To:* Kinney, Michael D <michael.d.kinney@intel.com
> >>>>> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io
> >>>>> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com
> >>>>> <mailto:lersek@redhat.com>>; Kubacki, Michael
> >>>>> <michael.kubacki@microsoft.com
> >>>>> <mailto:michael.kubacki@microsoft.com>>
> >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification
> >>>>> PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>>>> condensed arguments
> >>>>>
> >>>>> [AMD Official Use Only - General]
> >>>>>
> >>>>> Uncrustify can fix the first argument that is not at the indent
> >>>>> with two space. It also can fix the first argument that is not at
> >>>>> the new line.
> >>>>>
> >>>>> But it also makes each argument a new line if multiple args are
> >>>>> condensed in one line. That is what we have to update Uncrustify
> >>>>> if we have this patch merged to CCS.
> >>>>>
> >>>>> +Michael Kubacki in loop.
> >>>>>
> >>>>> Abner
> >>>>>
> >>>>> *From:* Kinney, Michael D <michael.d.kinney@intel.com
> >>>>> <mailto:michael.d.kinney@intel.com>>
> >>>>> *Sent:* Sunday, November 13, 2022 9:58 AM
> >>>>> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang,
> >>>>> Abner <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>;
> Laszlo
> >>>>> Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney,
> >>>>> Michael D <michael.d.kinney@intel.com
> >>>>> <mailto:michael.d.kinney@intel.com>>
> >>>>> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification
> >>>>> PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>>>> condensed arguments
> >>>>>
> >>>>>
> >>>>>
> >>>>> *Caution:*This message originated from an External Source. Use
> >>>>> proper caution when opening attachments, clicking links, or
> responding.
> >>>>>
> >>>>> Is this exactly what Uncrustify does now?
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf
> Of
> >>>>> *Chang, Abner via groups.io
> >>>>> *Sent:* Saturday, November 12, 2022 5:36 PM
> >>>>> *To:* Laszlo Ersek <lersek@redhat.com
> <mailto:lersek@redhat.com>>;
> >>>>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >>>>> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification
> >>>>> PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow
> >>>>> condensed arguments
> >>>>>
> >>>>> Hi all,
> >>>>> As we are going to release CCS 2.3, we would like to address some
> >>>>> pending issues of CCS. For this, I think we can,
> >>>>> - Still keep the one line per argument style in CCS although the
> >>>>> multi-arguments in the one line style can cover this. This avoids
> >>>>> confusion from readers and questions about if they can do the
> >>>>> one-line per argument style.
> >>>>> - If the arguments are in different lines, the first argument must
> >>>>> be indented with two spaces from the start of the function name or
> >>>>> the member function name.
> >>>>> How is this?
> >>>>>
> >>>>> Abner
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >
> >
> > 
> >
> >

Re: [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[Kinney, Michael D]

Laszlo,

You can also provide links to the GitHub commits with "?w=1"
flag appended.  If you select the "Display the rich diff" button
above each changed file, GitHub renders a view with change bars.

https://github.com/lersek/edk2-CCodingStandardsSpecification/commit/2c5534a24b15616fdaa02478858ed1d8908dc653?w=1

https://github.com/lersek/edk2-CCodingStandardsSpecification/commit/e3797dc48316052005cefa26246ab2fd32641881?w=1

Best regards,

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 11, 2017 9:49 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [edk2-CCodingStandardsSpecification PATCH 0/2]
> improvements related to line wrapping
> 
> We've discussed these ideas repeatedly over time; I'm now
> attempting to
> formalize them. Also test-driving the documentation contribution
> process.
> 
> - Repo: https://github.com/lersek/edk2-
> CCodingStandardsSpecification.git
> - Branch: line_wrapping
> - Rendered views of the pages modified:
>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> coding-standards-sp/content/v/line_wrapping/#edk-ii-c-coding-
> standards-specification
>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> coding-standards-sp/content/v/line_wrapping/5_source_files/#51-
> general-rules
>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> coding-standards-
> sp/content/v/line_wrapping/5_source_files/52_spacing.html#52-
> spacing
> 
> 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>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   Source Files / General Rules: limit line lengths to 80 columns
>   Source Files / Spacing / Multi-line func. calls: allow
> condensed
>     arguments
> 
>  5_source_files/52_spacing.md | 29 +++++++++++++++++++-
>  5_source_files/README.md     | 17 ++++++++----
>  README.md                    |  2 ++
>  3 files changed, 42 insertions(+), 6 deletions(-)
> 
> --
> 2.13.1.3.g8be5a757fa67

Re: [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[Laszlo Ersek]

Mike,

On 08/11/17 19:07, Kinney, Michael D wrote:
> Laszlo,
> 
> You can also provide links to the GitHub commits with "?w=1"
> flag appended.  If you select the "Display the rich diff" button
> above each changed file, GitHub renders a view with change bars.
> 
> https://github.com/lersek/edk2-CCodingStandardsSpecification/commit/2c5534a24b15616fdaa02478858ed1d8908dc653?w=1

I opened two browser tabs with this link, one without "?w=1" and another
with "?w=1", and compared the contents visually (by switching back and
forth between the tabs), checking each screen-ful. I'm not seeing any
differences.

When I click the button that you mention, I do get a rendered diff as
well. I wonder if this feature is javascript-only, or if it can be
triggered with URL changes.

Thanks
Laszlo

> 
> https://github.com/lersek/edk2-CCodingStandardsSpecification/commit/e3797dc48316052005cefa26246ab2fd32641881?w=1
> 
> Best regards,
> 
> Mike
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, August 11, 2017 9:49 AM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: [edk2-CCodingStandardsSpecification PATCH 0/2]
>> improvements related to line wrapping
>>
>> We've discussed these ideas repeatedly over time; I'm now
>> attempting to
>> formalize them. Also test-driving the documentation contribution
>> process.
>>
>> - Repo: https://github.com/lersek/edk2-
>> CCodingStandardsSpecification.git
>> - Branch: line_wrapping
>> - Rendered views of the pages modified:
>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>> coding-standards-sp/content/v/line_wrapping/#edk-ii-c-coding-
>> standards-specification
>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>> coding-standards-sp/content/v/line_wrapping/5_source_files/#51-
>> general-rules
>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>> coding-standards-
>> sp/content/v/line_wrapping/5_source_files/52_spacing.html#52-
>> spacing
>>
>> 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>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (2):
>>   Source Files / General Rules: limit line lengths to 80 columns
>>   Source Files / Spacing / Multi-line func. calls: allow
>> condensed
>>     arguments
>>
>>  5_source_files/52_spacing.md | 29 +++++++++++++++++++-
>>  5_source_files/README.md     | 17 ++++++++----
>>  README.md                    |  2 ++
>>  3 files changed, 42 insertions(+), 6 deletions(-)
>>
>> --
>> 2.13.1.3.g8be5a757fa67
> 

Re: [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[Kinney, Michael D]

Laszlo,

If you only change a couple of words in a sentence, it will
show the differences at the work level instead of the line
level.  If you add/remove entire paragraphs, then you will 
not see any differences.  I am just in the habit of always
adding ?w=1 to provide the best possible review format.

I have not found a URL to auto select that view.  When a 
single patch changes several files, you get that button for
each file within the patch.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> Behalf Of Laszlo Ersek
> Sent: Tuesday, August 15, 2017 4:01 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-
> 01 <edk2-devel@lists.01.org>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [edk2-CCodingStandardsSpecification PATCH
> 0/2] improvements related to line wrapping
> 
> Mike,
> 
> On 08/11/17 19:07, Kinney, Michael D wrote:
> > Laszlo,
> >
> > You can also provide links to the GitHub commits with "?w=1"
> > flag appended.  If you select the "Display the rich diff"
> button
> > above each changed file, GitHub renders a view with change
> bars.
> >
> > https://github.com/lersek/edk2-
> CCodingStandardsSpecification/commit/2c5534a24b15616fdaa02478858
> ed1d8908dc653?w=1
> 
> I opened two browser tabs with this link, one without "?w=1" and
> another
> with "?w=1", and compared the contents visually (by switching
> back and
> forth between the tabs), checking each screen-ful. I'm not
> seeing any
> differences.
> 
> When I click the button that you mention, I do get a rendered
> diff as
> well. I wonder if this feature is javascript-only, or if it can
> be
> triggered with URL changes.
> 
> Thanks
> Laszlo
> 
> >
> > https://github.com/lersek/edk2-
> CCodingStandardsSpecification/commit/e3797dc48316052005cefa26246
> ab2fd32641881?w=1
> >
> > Best regards,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Friday, August 11, 2017 9:49 AM
> >> To: edk2-devel-01 <edk2-devel@lists.01.org>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen,
> Jordan L
> >> <jordan.l.justen@intel.com>; Leif Lindholm
> >> <leif.lindholm@linaro.org>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>
> >> Subject: [edk2-CCodingStandardsSpecification PATCH 0/2]
> >> improvements related to line wrapping
> >>
> >> We've discussed these ideas repeatedly over time; I'm now
> >> attempting to
> >> formalize them. Also test-driving the documentation
> contribution
> >> process.
> >>
> >> - Repo: https://github.com/lersek/edk2-
> >> CCodingStandardsSpecification.git
> >> - Branch: line_wrapping
> >> - Rendered views of the pages modified:
> >>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> >> coding-standards-sp/content/v/line_wrapping/#edk-ii-c-coding-
> >> standards-specification
> >>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> >> coding-standards-
> sp/content/v/line_wrapping/5_source_files/#51-
> >> general-rules
> >>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
> >> coding-standards-
> >> sp/content/v/line_wrapping/5_source_files/52_spacing.html#52-
> >> spacing
> >>
> >> 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>
> >>
> >> Thanks
> >> Laszlo
> >>
> >> Laszlo Ersek (2):
> >>   Source Files / General Rules: limit line lengths to 80
> columns
> >>   Source Files / Spacing / Multi-line func. calls: allow
> >> condensed
> >>     arguments
> >>
> >>  5_source_files/52_spacing.md | 29 +++++++++++++++++++-
> >>  5_source_files/README.md     | 17 ++++++++----
> >>  README.md                    |  2 ++
> >>  3 files changed, 42 insertions(+), 6 deletions(-)
> >>
> >> --
> >> 2.13.1.3.g8be5a757fa67
> >
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to line wrapping

[Laszlo Ersek]

On 08/15/17 17:17, Kinney, Michael D wrote:
> Laszlo,
> 
> If you only change a couple of words in a sentence, it will
> show the differences at the work level instead of the line
> level.

This crossed my mind, but when I compared the two views, I saw the
word-level highlighting (on top of the line-level highlighting) even
under the URL without the w=1 parameter. I was confused.

> If you add/remove entire paragraphs, then you will 
> not see any differences.  I am just in the habit of always
> adding ?w=1 to provide the best possible review format.
> 
> I have not found a URL to auto select that view.  When a 
> single patch changes several files, you get that button for
> each file within the patch.

I'll try to remember adding such URLs to the blurb that require the
least amount of clicks in order to end up with a "rich diff".

Thanks!
Laszlo

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
>> Behalf Of Laszlo Ersek
>> Sent: Tuesday, August 15, 2017 4:01 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-
>> 01 <edk2-devel@lists.01.org>
>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2] [edk2-CCodingStandardsSpecification PATCH
>> 0/2] improvements related to line wrapping
>>
>> Mike,
>>
>> On 08/11/17 19:07, Kinney, Michael D wrote:
>>> Laszlo,
>>>
>>> You can also provide links to the GitHub commits with "?w=1"
>>> flag appended.  If you select the "Display the rich diff"
>> button
>>> above each changed file, GitHub renders a view with change
>> bars.
>>>
>>> https://github.com/lersek/edk2-
>> CCodingStandardsSpecification/commit/2c5534a24b15616fdaa02478858
>> ed1d8908dc653?w=1
>>
>> I opened two browser tabs with this link, one without "?w=1" and
>> another
>> with "?w=1", and compared the contents visually (by switching
>> back and
>> forth between the tabs), checking each screen-ful. I'm not
>> seeing any
>> differences.
>>
>> When I click the button that you mention, I do get a rendered
>> diff as
>> well. I wonder if this feature is javascript-only, or if it can
>> be
>> triggered with URL changes.
>>
>> Thanks
>> Laszlo
>>
>>>
>>> https://github.com/lersek/edk2-
>> CCodingStandardsSpecification/commit/e3797dc48316052005cefa26246
>> ab2fd32641881?w=1
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>> Sent: Friday, August 11, 2017 9:49 AM
>>>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen,
>> Jordan L
>>>> <jordan.l.justen@intel.com>; Leif Lindholm
>>>> <leif.lindholm@linaro.org>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>> Subject: [edk2-CCodingStandardsSpecification PATCH 0/2]
>>>> improvements related to line wrapping
>>>>
>>>> We've discussed these ideas repeatedly over time; I'm now
>>>> attempting to
>>>> formalize them. Also test-driving the documentation
>> contribution
>>>> process.
>>>>
>>>> - Repo: https://github.com/lersek/edk2-
>>>> CCodingStandardsSpecification.git
>>>> - Branch: line_wrapping
>>>> - Rendered views of the pages modified:
>>>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>>>> coding-standards-sp/content/v/line_wrapping/#edk-ii-c-coding-
>>>> standards-specification
>>>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>>>> coding-standards-
>> sp/content/v/line_wrapping/5_source_files/#51-
>>>> general-rules
>>>>   - https://lersek.gitbooks.io/laszlo-s-fork-of-the-edk-ii-c-
>>>> coding-standards-
>>>> sp/content/v/line_wrapping/5_source_files/52_spacing.html#52-
>>>> spacing
>>>>
>>>> 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>
>>>>
>>>> Thanks
>>>> Laszlo
>>>>
>>>> Laszlo Ersek (2):
>>>>   Source Files / General Rules: limit line lengths to 80
>> columns
>>>>   Source Files / Spacing / Multi-line func. calls: allow
>>>> condensed
>>>>     arguments
>>>>
>>>>  5_source_files/52_spacing.md | 29 +++++++++++++++++++-
>>>>  5_source_files/README.md     | 17 ++++++++----
>>>>  README.md                    |  2 ++
>>>>  3 files changed, 42 insertions(+), 6 deletions(-)
>>>>
>>>> --
>>>> 2.13.1.3.g8be5a757fa67
>>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel