Re: [PATCH v2 00/33] qapi: docs: width=70 and two spaces between sentences

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 29.10.25 11:29, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>>> On Sat, Oct 11, 2025 at 05:04:06PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all!
>>>>
>>>> Let's bring the documentation in line with the requirements. And
>>>> do check these requirements in QAPI parser.
>>>
>>> This implicitly assumes that the requirements are desirable.
>>>
>>> This is a large number of patches, showing the requirements are widely
>>> ignored today. When I look at the changes in the patches my overwhealming
>>> reaction is that they are not beneficial, which in turn makes me believe
>>> the requirements should be changed to match the reality of the code,
>>> rather than the reverse.
>>
>> A QAPI schema contains four distinct kinds of text:
>>
>> 1. Schema code
>>
>> 2. Example code in comments
>>
>> 3. Doc comments less example code, i.e. prose
>>
>> 4. Non-doc comments
>>
>> This series touches all four.
>
> I'm unsure about [1.]. What do you mean? The series touch only comments.
>
> I now check:
>
>  git diff 37137ae582 HEAD | grep '^[+-][^#+-]'
>
> where 37137ae582 is first commit "qapi: Add documentation format validation"
> for me, and this grep finds nothing..

You're right.  Cross-eyed, I guess %-}

> Assume [4.] is a tiny part.

Yes.

>> "The requirements" refers to docs/devel/qapi-code-gen.rst section
>> Documentation comments / Documentation markup:
>>
>>      For legibility, wrap text paragraphs so every line is at most 70
>>      characters long.
>>
>>      Separate sentences with two spaces.
>>
>> I've explained why these rules make sense a number of times, and I'm
>> happy to explain again if needed.
>>
>> Note this applies only to doc comments.
>>
>> I've been enforcing it manually for prose.  Whether it should be
>> enforced for example code is debatable.  Let's focus on prose.
>>
>> "Widely ignored" is not true, and I have numbers to back that up.
>>
>> We have some 20,000 lines of doc comments in the main QAPI schema and
>> the QGA QAPI schema.  Some 3,000 lines are examples.  That leaves a bit
>> over 17,000 lines of prose in 48 files.
>>
>> If I drop the changes to the other three kinds from Vladimir's series,
>> and add a few more prose changes he missed
>
> Hmm it surprises me.. Does it mean that the check added in patch 01 misses
> some violations?

I found a few more paragraphs to reflow for reasons other than long
lines, a few extra blank lines, a few missing blank lines, and slightly
off indentation in a few places.  None of this I expect your code to
catch.

I found a few errors in your patch, like this one in PATCH 13:

    @@ -620,7 +622,8 @@
     ##
     # @NumaCpuOptions:
     #
    -# Option "-numa cpu" overrides default cpu to node mapping.  It accepts
    +# Option "-numa cpu" overrides default cpu to node mapping.  It
    +#     accepts
     # the same set of cpu properties as returned by
     # `query-hotpluggable-cpus[].props <query-hotpluggable-cpus>`, where
     # node-id could be used to override default node mapping.
    @@ -686,7 +689,8 @@

They made me feel useful ;)

I also found a few single spaces that should be double.  Maybe the code
could be improved to catch them.

>>, I get this diffstat:
>>   24 files changed, 351 insertions(+), 332 deletions(-)
>
> Compare with original diffstat:
>
>  33 files changed, 713 insertions(+), 704 deletions(-)
>
> So obviously, touching up code examples makes the series twice more invasive.
>
> I agree, to at least postpone examples changing. And the series show, that in many
> cases there are no obvious possibility to satisfy restrictions for examples.
>
> Hmm, probably, we want another limit for examples? 90 characters? Anyway, not in this
> series.

I figure code in examples should be treated just like "real" code.

Avoiding long prose lines is easy.  When I reflowed the entire QAPI
schema documentation to stay within the limit (commit a937b6aa739), not
a single line break was awkward.

Code is unlike prose: it's often more deeply indented, and it contains
more longer words (identifiers).  Because of that, a long code line can
be less bad than an awkward line break.  Use your judgement.

devel/style.rst thus advises:

    Line width
    ==========

    Lines should be 80 characters; try not to make them longer.

    Sometimes it is hard to do, especially when dealing with QEMU subsystems
    that use long function or symbol names. If wrapping the line at 80 columns
    is obviously less readable and more awkward, prefer not to wrap it; better
    to have an 85 character line than one which is awkwardly wrapped.

    Even in that case, try not to make lines much longer than 80 characters.
    (The checkpatch script will warn at 100 characters, but this is intended
    as a guard against obviously-overlength lines, not a target.)