[PATCH] .editorconfig: set max line at 70 chars for QAPI files

marcandre.lureau@redhat.com posted 1 patch 1 year, 1 month ago
.editorconfig | 1 +
1 file changed, 1 insertion(+)
[PATCH] .editorconfig: set max line at 70 chars for QAPI files
Posted by marcandre.lureau@redhat.com 1 year, 1 month ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

This seems to be the preferred style.

The EditorConfig property is not supported by all editors:
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 .editorconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.editorconfig b/.editorconfig
index 7303759ed7..8c5ebc6a1b 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -47,3 +47,4 @@ emacs_mode = glsl
 [*.json]
 indent_style = space
 emacs_mode = python
+max_line_length = 70
-- 
2.39.2


Re: [PATCH] .editorconfig: set max line at 70 chars for QAPI files
Posted by Marc-André Lureau 1 year, 1 month ago
Hi

On Tue, Mar 7, 2023 at 4:32 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This seems to be the preferred style.
>
> The EditorConfig property is not supported by all editors:
> https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  .editorconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.editorconfig b/.editorconfig
> index 7303759ed7..8c5ebc6a1b 100644
> --- a/.editorconfig
> +++ b/.editorconfig
> @@ -47,3 +47,4 @@ emacs_mode = glsl
>  [*.json]
>  indent_style = space
>  emacs_mode = python
> +max_line_length = 70

ack or nack ?
Re: [PATCH] .editorconfig: set max line at 70 chars for QAPI files
Posted by Markus Armbruster 1 year, 1 month ago
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Tue, Mar 7, 2023 at 4:32 PM <marcandre.lureau@redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> This seems to be the preferred style.
>>
>> The EditorConfig property is not supported by all editors:
>> https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  .editorconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/.editorconfig b/.editorconfig
>> index 7303759ed7..8c5ebc6a1b 100644
>> --- a/.editorconfig
>> +++ b/.editorconfig
>> @@ -47,3 +47,4 @@ emacs_mode = glsl
>>  [*.json]
>>  indent_style = space
>>  emacs_mode = python
>> +max_line_length = 70
>
> ack or nack ?

I think we should first address the doc syntax misfeature that pushes us
to the right, and clean up existing overlong lines.  Can't say how hard
the former would be, so I'm having a look.
Re: [PATCH] .editorconfig: set max line at 70 chars for QAPI files
Posted by Daniel P. Berrangé 1 year, 1 month ago
On Tue, Mar 07, 2023 at 04:30:27PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This seems to be the preferred style.
> 
> The EditorConfig property is not supported by all editors:
> https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  .editorconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.editorconfig b/.editorconfig
> index 7303759ed7..8c5ebc6a1b 100644
> --- a/.editorconfig
> +++ b/.editorconfig
> @@ -47,3 +47,4 @@ emacs_mode = glsl
>  [*.json]
>  indent_style = space
>  emacs_mode = python
> +max_line_length = 70

Why 70 as a hard limit ?  I thought QEMU policy was that 80 was a soft
limit and we were happy with 90 if it avoided wrapping that would hurt
readability. 

With 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] .editorconfig: set max line at 70 chars for QAPI files
Posted by Markus Armbruster 1 year, 1 month ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Mar 07, 2023 at 04:30:27PM +0400, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> 
>> This seems to be the preferred style.
>> 
>> The EditorConfig property is not supported by all editors:
>> https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
>> 
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  .editorconfig | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/.editorconfig b/.editorconfig
>> index 7303759ed7..8c5ebc6a1b 100644
>> --- a/.editorconfig
>> +++ b/.editorconfig
>> @@ -47,3 +47,4 @@ emacs_mode = glsl
>>  [*.json]
>>  indent_style = space
>>  emacs_mode = python
>> +max_line_length = 70
>
> Why 70 as a hard limit ?  I thought QEMU policy was that 80 was a soft
> limit and we were happy with 90 if it avoided wrapping that would hurt
> readability. 

We have the rule because we're tired of arguing about readability of
long lines all the time.  But readability should trump rules!

Long lines are hard to read.  It's not a matter of window width.  Humans
tend to have trouble following long lines with their eyes (I sure do).
Typographic manuals suggest to limit columns to roughly 60 characters
for exactly that reason[*].

Wider windows do help with heavily indented material, as long as the
text itself isn't too wide.

Occasionally, there's simply no good way to break a long line without
making it *less* readable.  We should accept the long line as the lesser
evil then.

The QAPI schema files consist of ~4,000 lines of code, ~18,000 lines of
doc comments, and ~1000 blank lines.

1150 lines exceed 75 characters.  271 exceed 80 characters.  Mostly doc
strings, and mostly due to carelessness, not necessity.

Some doc strings are heavily indented[**], and letting these go over the
limit can be okay.

We should employ good taste.  Unfortunately, that seems to be in short
supply.  Rules are a poor substitute, but here we are.

When I review QAPI schema patches, I flag unnecessary long lines.  I
don't intend to stop that :)


[*] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style

[**] Caused by unfortunate doc string syntax; wish I had the time to fix
it
Re: [PATCH] .editorconfig: set max line at 70 chars for QAPI files
Posted by Marc-André Lureau 1 year, 1 month ago
Hi

On Tue, Mar 7, 2023 at 4:41 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 07, 2023 at 04:30:27PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > This seems to be the preferred style.
> >
> > The EditorConfig property is not supported by all editors:
> > https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  .editorconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/.editorconfig b/.editorconfig
> > index 7303759ed7..8c5ebc6a1b 100644
> > --- a/.editorconfig
> > +++ b/.editorconfig
> > @@ -47,3 +47,4 @@ emacs_mode = glsl
> >  [*.json]
> >  indent_style = space
> >  emacs_mode = python
> > +max_line_length = 70
>
> Why 70 as a hard limit ?  I thought QEMU policy was that 80 was a soft
> limit and we were happy with 90 if it avoided wrapping that would hurt
> readability.

Markus regularly point out lines over 70 characters:
https://patchew.org/QEMU/20230306122751.2355515-1-marcandre.lureau@redhat.com/20230306122751.2355515-9-marcandre.lureau@redhat.com/#871qm1j2zc.fsf@pond.sub.org

(my default emacs config has fill-column 80, and I use fill-paragraph
- although sometime I may forget it)