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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
> >
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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 > > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
>>
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.