[PATCH] coding-style: Document 80 chars limit for line length

Michal Privoznik posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/bf37b9e438f32e1f9e3c8659b0bebecd04678efd.1606741104.git.mprivozn@redhat.com
docs/coding-style.rst | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[PATCH] coding-style: Document 80 chars limit for line length
Posted by Michal Privoznik 3 years, 5 months ago
The idea is to have it like a soft limit: if possible then break
lines, if not then have a long line instead of some creative
approach.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/coding-style.rst | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index cfd7b16638..813128bfb6 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -131,7 +131,7 @@ around operators and keywords:
 
   indent-libvirt()
   {
-    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
+    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l80 -lc80 \
            -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
            --no-tabs "$@"
   }
@@ -141,6 +141,18 @@ further, by piping it through ``expand -i``, since some leading
 TABs can get through. Usually they're in macro definitions or
 strings, and should be converted anyhow.
 
+The recommended length for lines is 80 characters, but common sense
+should prevail. It may get tricky around some names (because of how
+Libvirt constructs names for functions/enums/etc.)
+
+::
+
+  foo(
+  arg1, arg2
+  );            // Bad
+  foo(arg1,
+      arg2);    // Good
+
 Libvirt requires a C99 compiler for various reasons. However, most
 of the code base prefers to stick to C89 syntax unless there is a
 compelling reason otherwise. For example, it is preferable to use
-- 
2.26.2

Re: [PATCH] coding-style: Document 80 chars limit for line length
Posted by Daniel P. Berrangé 3 years, 4 months ago
On Mon, Nov 30, 2020 at 01:58:24PM +0100, Michal Privoznik wrote:
> The idea is to have it like a soft limit: if possible then break
> lines, if not then have a long line instead of some creative
> approach.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  docs/coding-style.rst | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/coding-style.rst b/docs/coding-style.rst
> index cfd7b16638..813128bfb6 100644
> --- a/docs/coding-style.rst
> +++ b/docs/coding-style.rst
> @@ -131,7 +131,7 @@ around operators and keywords:
>  
>    indent-libvirt()
>    {
> -    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
> +    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l80 -lc80 \

The indent tool enforces line length no matter what....

>             -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
>             --no-tabs "$@"
>    }
> @@ -141,6 +141,18 @@ further, by piping it through ``expand -i``, since some leading
>  TABs can get through. Usually they're in macro definitions or
>  strings, and should be converted anyhow.
>  
> +The recommended length for lines is 80 characters, but common sense
> +should prevail. It may get tricky around some names (because of how
> +Libvirt constructs names for functions/enums/etc.)

but this is a mere recommendation.

IMHO we should say

 "The maximum permitted line length is 100 characters, but lines
  should aim to be approximately 80 characters."

and then use -l100 for indent


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] coding-style: Document 80 chars limit for line length
Posted by Michal Privoznik 3 years, 4 months ago
On 12/2/20 11:52 AM, Daniel P. Berrangé wrote:
> On Mon, Nov 30, 2020 at 01:58:24PM +0100, Michal Privoznik wrote:
>> The idea is to have it like a soft limit: if possible then break
>> lines, if not then have a long line instead of some creative
>> approach.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   docs/coding-style.rst | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/coding-style.rst b/docs/coding-style.rst
>> index cfd7b16638..813128bfb6 100644
>> --- a/docs/coding-style.rst
>> +++ b/docs/coding-style.rst
>> @@ -131,7 +131,7 @@ around operators and keywords:
>>   
>>     indent-libvirt()
>>     {
>> -    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
>> +    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l80 -lc80 \
> 
> The indent tool enforces line length no matter what....

Yeah, it's not perfect, but I am no friend with gnu indent so I don't 
know how to specify hard and soft limits and quick skim through manpage 
did not suggest it's possible.

> 
>>              -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
>>              --no-tabs "$@"
>>     }
>> @@ -141,6 +141,18 @@ further, by piping it through ``expand -i``, since some leading
>>   TABs can get through. Usually they're in macro definitions or
>>   strings, and should be converted anyhow.
>>   
>> +The recommended length for lines is 80 characters, but common sense
>> +should prevail. It may get tricky around some names (because of how
>> +Libvirt constructs names for functions/enums/etc.)
> 
> but this is a mere recommendation.
> 
> IMHO we should say
> 
>   "The maximum permitted line length is 100 characters, but lines
>    should aim to be approximately 80 characters."
> 
> and then use -l100 for indent

Works for me. Thomas, since you suggested we document this, does this 
wording sound reasonable to you? If so, I will post v2.

Michal

Re: [PATCH] coding-style: Document 80 chars limit for line length
Posted by Thomas Huth 3 years, 4 months ago
On 02/12/2020 12.20, Michal Privoznik wrote:
> On 12/2/20 11:52 AM, Daniel P. Berrangé wrote:
>> On Mon, Nov 30, 2020 at 01:58:24PM +0100, Michal Privoznik wrote:
>>> The idea is to have it like a soft limit: if possible then break
>>> lines, if not then have a long line instead of some creative
>>> approach.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>   docs/coding-style.rst | 14 +++++++++++++-
>>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/docs/coding-style.rst b/docs/coding-style.rst
>>> index cfd7b16638..813128bfb6 100644
>>> --- a/docs/coding-style.rst
>>> +++ b/docs/coding-style.rst
>>> @@ -131,7 +131,7 @@ around operators and keywords:
>>>       indent-libvirt()
>>>     {
>>> -    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 \
>>> +    indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l80 -lc80 \
>>
>> The indent tool enforces line length no matter what....
> 
> Yeah, it's not perfect, but I am no friend with gnu indent so I don't know
> how to specify hard and soft limits and quick skim through manpage did not
> suggest it's possible.
> 
>>
>>>              -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc \
>>>              --no-tabs "$@"
>>>     }
>>> @@ -141,6 +141,18 @@ further, by piping it through ``expand -i``, since
>>> some leading
>>>   TABs can get through. Usually they're in macro definitions or
>>>   strings, and should be converted anyhow.
>>>   +The recommended length for lines is 80 characters, but common sense
>>> +should prevail. It may get tricky around some names (because of how
>>> +Libvirt constructs names for functions/enums/etc.)
>>
>> but this is a mere recommendation.
>>
>> IMHO we should say
>>
>>   "The maximum permitted line length is 100 characters, but lines
>>    should aim to be approximately 80 characters."
>>
>> and then use -l100 for indent
> 
> Works for me. Thomas, since you suggested we document this, does this
> wording sound reasonable to you? If so, I will post v2.

Yes, I think using -l100 for indent and saying that 80 is preferred is
better! Thanks for tackling this!

 Thomas