diff mbox series

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

Message ID 20230307123027.2485499-1-marcandre.lureau@redhat.com
State New
Headers show
Series .editorconfig: set max line at 70 chars for QAPI files | expand

Commit Message

Marc-André Lureau March 7, 2023, 12:30 p.m. UTC 
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(+)

Comments

Daniel P. Berrangé March 7, 2023, 12:40 p.m. UTC | #1 
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
Marc-André Lureau March 7, 2023, 12:55 p.m. UTC | #2 
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)
Markus Armbruster March 7, 2023, 1:09 p.m. UTC | #3 
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
Marc-André Lureau March 21, 2023, 7:30 a.m. UTC | #4 
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 ?
Markus Armbruster March 21, 2023, 1:53 p.m. UTC | #5 
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.
diff mbox series

Patch

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