[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/17] qapi: Relax doc string @name: description indentation
From: |
Juan Quintela |
Subject: |
Re: [PATCH 13/17] qapi: Relax doc string @name: description indentation rules |
Date: |
Fri, 28 Apr 2023 20:25:59 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> wrote:
> The QAPI schema doc comment language provides special syntax for
> command and event arguments, struct and union members, alternate
> branches, enumeration values, and features: descriptions starting with
> "@name:".
>
> By convention, we format them like this:
>
> # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
> # sed do eiusmod tempor incididunt ut labore et dolore
> # magna aliqua.
>
> Okay for names as short as "name", but we have much longer ones. Their
> description gets squeezed against the right margin, like this:
>
> # @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization
> could
> # not avoid copying dirty pages. This is
> between
> # 0 and @dirty-sync-count *
> @multifd-channels.
> # (since 7.1)
>
> The description text is effectively just 50 characters wide. Easy
> enough to read, but can be cumbersome to write.
>
> The awkward squeeze against the right margin makes people go beyond it,
> which produces two undesirables: arguments about style, and descriptions
> that are unnecessarily hard to read, like this one:
>
> # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.
> This is
> # only present when the postcopy-blocktime
> migration capability
> # is enabled. (Since 3.0)
>
> We could instead format it like
>
> # @postcopy-vcpu-blocktime:
> # list of the postcopy blocktime per vCPU. This is only present
> # when the postcopy-blocktime migration capability is
> # enabled. (Since 3.0)
>
> or, since the commit before previous, like
>
> # @postcopy-vcpu-blocktime:
> # list of the postcopy blocktime per vCPU. This is only present
> # when the postcopy-blocktime migration capability is
> # enabled. (Since 3.0)
>
> However, I'd rather have
>
> # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.
> # This is only present when the postcopy-blocktime migration
> # capability is enabled. (Since 3.0)
>
> because this is how rST field and option lists work.
>
> To get this, we need to let the first non-blank line after the
> "@name:" line determine expected indentation.
>
> This fills up the indentation pitfall mentioned in
> docs/devel/qapi-code-gen.rst. A related pitfall still exists. Update
> the text to show it.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
> ---
> docs/devel/qapi-code-gen.rst | 10 ++--
> scripts/qapi/parser.py | 73 +++++++--------------------
> tests/qapi-schema/doc-bad-indent.err | 2 +-
> tests/qapi-schema/doc-bad-indent.json | 3 +-
bad order of files
> tests/qapi-schema/doc-good.json | 3 +-
> tests/qapi-schema/doc-good.out | 3 +-
good order of files
Should we tweak orderfiles so it displays first the json, and the err or
out files. reviewing json and then output makes things (at least to me)
simpler.
- [PATCH 00/17] qapi: Reformat doc comments, Markus Armbruster, 2023/04/28
- [PATCH 02/17] docs/devel/qapi-code-gen: Turn FIXME admonitions into comments, Markus Armbruster, 2023/04/28
- [PATCH 13/17] qapi: Relax doc string @name: description indentation rules, Markus Armbruster, 2023/04/28
- Re: [PATCH 13/17] qapi: Relax doc string @name: description indentation rules,
Juan Quintela <=
- [PATCH 11/17] qapi: Fix argument description indentation stripping, Markus Armbruster, 2023/04/28
- [PATCH 04/17] meson: Fix to make QAPI generator output depend on main.py, Markus Armbruster, 2023/04/28
- [PATCH 07/17] qapi: Tidy up a slightly awkward TODO comment, Markus Armbruster, 2023/04/28
- [PATCH 14/17] qapi: Section parameter @indent is no longer used, drop, Markus Armbruster, 2023/04/28
- [PATCH 03/17] qapi: Fix crash on stray double quote character, Markus Armbruster, 2023/04/28