[PATCH] Add line-wrapping guidelines to the coding style documentation

[PATCH] Add line-wrapping guidelines to the coding style documentation

[Wincent Colaiuta]

Help new contributors by providing some advice about line-wrapping; the
advice basically boils down to "use 80-characters minus some slop as a
rule of thumb", but also "use common sense", and "avoid gratuitous
rewrapping".

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---

The advice is based on the following post by Junio:

El 14/11/2007, a las 10:27, Junio C Hamano escribió:

> I did not mean to say 70 was the official limit.  Indeed, there
> is no hard official limit (and there shouldn't be any "hard"
> limit).  But 70 should certainly be lower than the limit anybody
> around here would want to impose, and that was why I said 70.
>
> Some considerations:
>
> - Many of us read the unformatted ASCII version, as AsciiDoc
>   was designed to be very readable, and mixing excessively long
>   and short lines will make the document harder to read.
>
> - We tend to exchange patches over e-mail and assume everybody
>   has at least 80-column wide terminals, so although we say a
>   line should be less than 80-chars, in practice the limit is
>   somewhat lower. to accomodate diff change marks [-+ ] and
>   quoting ">> " in e-mails.
>
> - But "80-chars minus a bit of slop" is not a very hard limit.
>
> Please apply some common sense to decide when to re-wrap and
> when not to within these constraints.


I also whipped up this quick Ruby script to print a histogram of line  
widths in the tree. You would run it like this to get a feel for the  
typical line lengths of the different types of file in the tree:

check-column-widths.rb *.c *.h
check-column-widths.rb *.sh
check-column-widths.rb *.perl
check-column-widths.rb Documentation/*.txt

A statistician could probably make some interesting comments about the  
results, but the basic trend is that, while there are plenty of  
examples of isolated long lines in the source tree (the longest is a  
287-character line in one of the perl scripts), the frequency starts  
to drop off pretty rapidly once you pass 70 columns and start climbing  
towards 80.

#!/usr/bin/env ruby

extensions = {}

# count frequency of line widths
ARGV.each do |arg|
   f = File.new arg
   extname = File.extname arg
   unless file_type = extensions[extname]
     extensions[extname] = file_type = {}
   end
   f.readlines.each do |line|
     len = line.chomp.length
     count = (file_type[len] or 0)
     file_type[len] = count + 1
   end
end

# print results
extensions.each do |file_type, frequencies|
   puts "Frequencies for #{file_type} files"

   # find longest line, most frequent line width, total line count
   longest       = frequencies.max { |a, b| a[0] <=> b[0] }[0]
   most_frequent = frequencies.max { |a, b| a[1] <=> b[1] }[1]
   total_lines   = frequencies.inject(0) { |sum, a| sum + a[1] }

   # draw histogram
   for i in 0..longest
     next if frequencies[i].nil?
     puts "%4d [%4d]: %s" % [i, frequencies[i], '.' * (frequencies[i]  
* 60 / most_frequent)]
   end
   puts "Maximum length: #{longest}"
   puts "   Total lines: #{total_lines}"
end

  Documentation/CodingGuidelines |   22 ++++++++++++++++++++++
  1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/ 
CodingGuidelines
index 3b042db..3fd698e 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -110,3 +110,25 @@ For C programs:
     used in the git core command set (unless your command is clearly
     separate from it, such as an importer to convert random-scm-X
     repositories to git).
+
+Line wrapping:
+
+ - While there are no official hard limits for line wrapping, we
+   generally try to keep shell scripts, C source files and AsciiDoc
+   documentation within the range of "80-characters minus some
+   slop".
+
+ - We assume that everyone has terminals that are at least 80
+   columns wide.
+
+ - In practice, we try to keep lines somewhat narrower than 80
+   columns to accommodate diff change marks [-+ ] and quoting ">> "
+   in emails.
+
+ - In the case of documentation, mixing excessively long and short
+   lines may make the AsciiDoc source harder to read, so try to
+   keep line lengths consistent.
+
+ - When submitting patches use common sense to decide whether to
+   rewrap, avoiding gratuitous changes.
+
-- 
1.5.3.5

Re: [PATCH] Add line-wrapping guidelines to the coding style documentation

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> A statistician could probably make some interesting comments about the  
> results, but the basic trend is that, while there are plenty of  
> examples of isolated long lines in the source tree (the longest is a  
> 287-character line in one of the perl scripts), the frequency starts  
> to drop off pretty rapidly once you pass 70 columns and start climbing  
> towards 80.

Gaah.  287???

> + - In the case of documentation, mixing excessively long and short
> +   lines may make the AsciiDoc source harder to read, so try to
> +   keep line lengths consistent.
> +
> + - When submitting patches use common sense to decide whether to
> +   rewrap, avoiding gratuitous changes.

Hmph.  The last item applies only to the documentation because
it uses the word "rewrap", but otherwise applies equally well to
the sources (re-indenting).  So probably "whether to rewrap or
reindent" would be a good change there.

Re: [PATCH] Add line-wrapping guidelines to the coding style documentation

[Johannes Schindelin]

Hi,

On Wed, 14 Nov 2007, Wincent Colaiuta wrote:

> Help new contributors by providing some advice about line-wrapping; the 
> advice basically boils down to "use 80-characters minus some slop as a 
> rule of thumb", but also "use common sense", and "avoid gratuitous 
> rewrapping".

We already have this:

 - We try to keep to at most 80 characters per line.

Besides, is it really necessary to be as explicit as you word it?  IOW is 
this patch needed?

Because if we go down that road, we might very well end up with a 
CodingGuidelines document that is larger than git's source code.

Ciao,
Dscho

[PATCH v3] Add line-wrapping guidelines to the coding style documentation

[Wincent Colaiuta]

Help new contributors by providing some advice about line-wrapping; the
advice basically boils down to "use 80 columns minus some slop as a
rule of thumb", but also "use common sense", and "avoid gratuitous
rewrapping".

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---

El 14/11/2007, a las 18:19, Johannes Schindelin escribió:

> On Wed, 14 Nov 2007, Wincent Colaiuta wrote:
>
>> Help new contributors by providing some advice about line-wrapping;  
>> the
>> advice basically boils down to "use 80-characters minus some slop  
>> as a
>> rule of thumb", but also "use common sense", and "avoid gratuitous
>> rewrapping".
>
> We already have this:
>
> - We try to keep to at most 80 characters per line.

Ah, didn't see that. It's in the "C programs" section and I was trying  
to provide a guideline that applied to all source types (given that  
this all started with a doc patch to an AsciiDoc source file).

> Besides, is it really necessary to be as explicit as you word it?   
> IOW is
> this patch needed?

I was basically just trying to help new people from making the same  
mistake that I did; ie. not knowing if there was an official limit,  
looking at the maximum line length in a file, making sure my patch  
didn't exceed that length (and re-wrapping to avoid exceeding it), and  
then getting reprimanded for gratuitous re-wrapping.

As for the explicitness, I was just paraphrasing the guidelines as  
Junio expressed them.

> Because if we go down that road, we might very well end up with a
> CodingGuidelines document that is larger than git's source code.

134 lines down (the current length of CodingGuidelines with that  
patch), about 100,000 lines to go (the rest of the codebase). So if we  
try very hard, we could indeed get there.

Here follows a revised patch which is more concise, and keeps all  
wrapping references at a single place. I lose your "at most 80  
characters" in favor of Junio's "80-characters minus some slop", and  
in fact state "80 columns" rather than "80 characters", because that's  
what we're really talking about, isn't it?

Cheers,
Wincent

  Documentation/CodingGuidelines |   13 +++++++++++--
  1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/ 
CodingGuidelines
index 3b042db..d2d1f32 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -58,8 +58,6 @@ For C programs:
   - We use tabs to indent, and interpret tabs as taking up to
     8 spaces.

- - We try to keep to at most 80 characters per line.
-
   - When declaring pointers, the star sides with the variable
     name, i.e. "char *string", not "char* string" or
     "char * string".  This makes it easier to understand code
@@ -110,3 +108,14 @@ For C programs:
     used in the git core command set (unless your command is clearly
     separate from it, such as an importer to convert random-scm-X
     repositories to git).
+
+Line wrapping:
+
+ - We generally try to keep scripts, C source files and AsciiDoc
+   documentation within the range of "80 columns minus some slop"
+   to accommodate diff change marks [-+ ] and quoting ">> " in
+   emails.
+
+ - When submitting patches use common sense to decide whether to
+   rewrap (or reindent), avoiding gratuitous changes.
+
-- 
1.5.3.5

Re: [PATCH v3] Add line-wrapping guidelines to the coding style documentation

[Andreas Ericsson]

Wincent Colaiuta wrote:
> Help new contributors by providing some advice about line-wrapping; the
> advice basically boils down to "use 80 columns minus some slop as a
> rule of thumb", but also "use common sense", and "avoid gratuitous
> rewrapping".
> 
> Signed-off-by: Wincent Colaiuta <win@wincent.com>
> ---
> 
> El 14/11/2007, a las 18:19, Johannes Schindelin escribió:
> 
>> On Wed, 14 Nov 2007, Wincent Colaiuta wrote:
>>
>>> Help new contributors by providing some advice about line-wrapping; the
>>> advice basically boils down to "use 80-characters minus some slop as a
>>> rule of thumb", but also "use common sense", and "avoid gratuitous
>>> rewrapping".
>>
>> We already have this:
>>
>> - We try to keep to at most 80 characters per line.
> 
> Ah, didn't see that. It's in the "C programs" section and I was trying 
> to provide a guideline that applied to all source types (given that this 
> all started with a doc patch to an AsciiDoc source file).
> 
>> Besides, is it really necessary to be as explicit as you word it?  IOW is
>> this patch needed?
> 
> I was basically just trying to help new people from making the same 
> mistake that I did; ie. not knowing if there was an official limit, 
> looking at the maximum line length in a file, making sure my patch 
> didn't exceed that length (and re-wrapping to avoid exceeding it), and 
> then getting reprimanded for gratuitous re-wrapping.
> 
> As for the explicitness, I was just paraphrasing the guidelines as Junio 
> expressed them.
> 
>> Because if we go down that road, we might very well end up with a
>> CodingGuidelines document that is larger than git's source code.
> 
> 134 lines down (the current length of CodingGuidelines with that patch), 
> about 100,000 lines to go (the rest of the codebase). So if we try very 
> hard, we could indeed get there.
> 
> Here follows a revised patch which is more concise, and keeps all 
> wrapping references at a single place. I lose your "at most 80 
> characters" in favor of Junio's "80-characters minus some slop", and in 
> fact state "80 columns" rather than "80 characters", because that's what 
> we're really talking about, isn't it?
> 
> Cheers,
> Wincent
> 
>  Documentation/CodingGuidelines |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/CodingGuidelines 
> b/Documentation/CodingGuidelines
> index 3b042db..d2d1f32 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -58,8 +58,6 @@ For C programs:
>   - We use tabs to indent, and interpret tabs as taking up to
>     8 spaces.
> 
> - - We try to keep to at most 80 characters per line.
> -
>   - When declaring pointers, the star sides with the variable
>     name, i.e. "char *string", not "char* string" or
>     "char * string".  This makes it easier to understand code
> @@ -110,3 +108,14 @@ For C programs:
>     used in the git core command set (unless your command is clearly
>     separate from it, such as an importer to convert random-scm-X
>     repositories to git).
> +
> +Line wrapping:
> +
> + - We generally try to keep scripts, C source files and AsciiDoc
> +   documentation

"We generally try to keep source and documentation" ... ?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH v3] Add line-wrapping guidelines to the coding style documentation

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> El 14/11/2007, a las 18:19, Johannes Schindelin escribió:
>
>> Besides, is it really necessary to be as explicit as you word it?
>> IOW is
>> this patch needed?
>
> I was basically just trying to help new people from making the same
> mistake that I did; ie. not knowing if there was an official limit,
> looking at the maximum line length in a file, making sure my patch
> didn't exceed that length (and re-wrapping to avoid exceeding it), and
> then getting reprimanded for gratuitous re-wrapping.

Sorry about that "reprimanded" part.  I should have been more
careful -- I did not really mean it that way.  It was more like
"it would have been nicer ... so please try to next time".

I know how stressful a life is for a contributor new to a
project.  You not only need to make sure the meat of your change
is good, but worry about the technicalities, such as the
presentation and style, the tone of the proposed log message
that addresses other peoples efforts you are building on; in
short, finding out the lay of the land, making yourself blend
well in the comminity.

We have a great way to quickly tell if somebody is new to the
project (i.e. "git shortlog --author=Wincent master").  I should
use it more often and adjust my tone accordingly.  It's only
fair for me to do so when the new person surely is trying hard
to do his part.

> diff --git a/Documentation/CodingGuidelines b/Documentation/
> CodingGuidelines
> index 3b042db..d2d1f32 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -58,8 +58,6 @@ For C programs:
>   - We use tabs to indent, and interpret tabs as taking up to
>     8 spaces.
>
> - - We try to keep to at most 80 characters per line.
> -
>   - When declaring pointers, the star sides with the variable
>     name, i.e. "char *string", not "char* string" or
>     "char * string".  This makes it easier to understand code

I agree that "80 columns" sits better in that new "Line
wrapping" section.  I wonder if "tabs are 8 spaces wide" also
belong there.

> @@ -110,3 +108,14 @@ For C programs:
>     used in the git core command set (unless your command is clearly
>     separate from it, such as an importer to convert random-scm-X
>     repositories to git).
> +
> +Line wrapping:
> +
> + - We generally try to keep scripts, C source files and AsciiDoc
> +   documentation within the range of "80 columns minus some slop"
> +   to accommodate diff change marks [-+ ] and quoting ">> " in
> +   emails.
> +
> + - When submitting patches use common sense to decide whether to
> +   rewrap (or reindent), avoiding gratuitous changes.
> +

Re: [PATCH v3] Add line-wrapping guidelines to the coding style documentation

[Wincent Colaiuta]

El 14/11/2007, a las 21:04, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> El 14/11/2007, a las 18:19, Johannes Schindelin escribió:
>>
>>> Besides, is it really necessary to be as explicit as you word it?
>>> IOW is
>>> this patch needed?
>>
>> I was basically just trying to help new people from making the same
>> mistake that I did; ie. not knowing if there was an official limit,
>> looking at the maximum line length in a file, making sure my patch
>> didn't exceed that length (and re-wrapping to avoid exceeding it),  
>> and
>> then getting reprimanded for gratuitous re-wrapping.
>
> Sorry about that "reprimanded" part.  I should have been more
> careful -- I did not really mean it that way.  It was more like
> "it would have been nicer ... so please try to next time".

Don't worry about it, Junio. I've been subscribed to the list for  
several months now, so I'm already aware of the perfectionism inherent  
in the process; I didn't take anything personally, and much less  
coming from you who is always diplomatic. I just thought, "this is  
good advice that Junio has posted to the list, I found it helpful, so  
I think it should go in the style doc".

And I'm no stranger to Johannes' scything sarcasm either, it's his  
personal style; luckily, along with the sarcasm he makes so many high- 
quality contributions to the community as well.

> I agree that "80 columns" sits better in that new "Line
> wrapping" section.  I wonder if "tabs are 8 spaces wide" also
> belong there.

You're probably right; I understand that the 8-wide tabs convention  
holds in shell and perl scripts too, not just in C source.

Cheers,
Wincent