[PATCH 01/22] qapi: Tidy up unusual line breaks

Markus Armbruster posted 22 patches 4 years, 4 months ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Eric Blake <eblake@redhat.com>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Eduardo Habkost <ehabkost@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Michael Roth <michael.roth@amd.com>
There is a newer version of this series
[PATCH 01/22] qapi: Tidy up unusual line breaks
Posted by Markus Armbruster 4 years, 4 months ago
Break lines between members instead of within members.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.rst            | 12 +++++------
 tests/qapi-schema/doc-good.json         |  4 ++--
 tests/qapi-schema/enum-if-invalid.json  |  4 ++--
 tests/qapi-schema/qapi-schema-test.json | 28 ++++++++++++-------------
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index ced7a5ffe1..b154eae82e 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -859,9 +859,9 @@ longhand form of MEMBER.
 Example: a struct type with unconditional member 'foo' and conditional
 member 'bar' ::
 
- { 'struct': 'IfStruct', 'data':
-   { 'foo': 'int',
-     'bar': { 'type': 'int', 'if': 'IFCOND'} } }
+ { 'struct': 'IfStruct',
+   'data': { 'foo': 'int',
+             'bar': { 'type': 'int', 'if': 'IFCOND'} } }
 
 A union's discriminator may not be conditional.
 
@@ -871,9 +871,9 @@ the longhand form of ENUM-VALUE_.
 Example: an enum type with unconditional value 'foo' and conditional
 value 'bar' ::
 
- { 'enum': 'IfEnum', 'data':
-   [ 'foo',
-     { 'name' : 'bar', 'if': 'IFCOND' } ] }
+ { 'enum': 'IfEnum',
+   'data': [ 'foo',
+             { 'name' : 'bar', 'if': 'IFCOND' } ] }
 
 Likewise, features can be conditional.  This requires the longhand
 form of FEATURE_.
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index e0027e4cf6..cbf5c56c4b 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -60,8 +60,8 @@
 #
 # @two is undocumented
 ##
-{ 'enum': 'Enum', 'data':
-  [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
+{ 'enum': 'Enum',
+  'data': [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
   'features': [ 'enum-feat' ],
   'if': 'IFCOND' }
 
diff --git a/tests/qapi-schema/enum-if-invalid.json b/tests/qapi-schema/enum-if-invalid.json
index 60bd0ef1d7..6bd20041f3 100644
--- a/tests/qapi-schema/enum-if-invalid.json
+++ b/tests/qapi-schema/enum-if-invalid.json
@@ -1,3 +1,3 @@
 # check invalid 'if' type
-{ 'enum': 'TestIfEnum', 'data':
-  [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
+{ 'enum': 'TestIfEnum',
+  'data': [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b6c36a9eee..3c43e14e22 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -220,27 +220,27 @@
 
 # test 'if' condition handling
 
-{ 'struct': 'TestIfStruct', 'data':
-  { 'foo': 'int',
-    'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
+{ 'struct': 'TestIfStruct',
+  'data': { 'foo': 'int',
+            'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
   'if': 'TEST_IF_STRUCT' }
 
-{ 'enum': 'TestIfEnum', 'data':
-  [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
+{ 'enum': 'TestIfEnum',
+  'data': [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
   'if': 'TEST_IF_ENUM' }
 
-{ 'union': 'TestIfUnion', 'data':
-  { 'foo': 'TestStruct',
-    'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
+{ 'union': 'TestIfUnion',
+  'data': { 'foo': 'TestStruct',
+            'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
   'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
 
 { 'command': 'test-if-union-cmd',
   'data': { 'union-cmd-arg': 'TestIfUnion' },
   'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
 
-{ 'alternate': 'TestIfAlternate', 'data':
-  { 'foo': 'int',
-    'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
+{ 'alternate': 'TestIfAlternate',
+  'data': { 'foo': 'int',
+            'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
   'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
 
 { 'command': 'test-if-alternate-cmd',
@@ -256,9 +256,9 @@
 
 { 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
 
-{ 'event': 'TEST_IF_EVENT', 'data':
-  { 'foo': 'TestIfStruct',
-    'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
+{ 'event': 'TEST_IF_EVENT',
+  'data': { 'foo': 'TestIfStruct',
+            'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
   'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
 
 { 'event': 'TEST_IF_EVENT2', 'data': {},
-- 
2.31.1


Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
Posted by Marc-André Lureau 4 years, 4 months ago
Hi

On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com> wrote:

> Break lines between members instead of within members.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I wish we would just automated tools to format files. With that git 2.23
feature, no more excuses :):
https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame

Sadly, our mixed json/py format doesn't get parsed after "blake" reformats
it (strings or trailing commas).

---
>  docs/devel/qapi-code-gen.rst            | 12 +++++------
>  tests/qapi-schema/doc-good.json         |  4 ++--
>  tests/qapi-schema/enum-if-invalid.json  |  4 ++--
>  tests/qapi-schema/qapi-schema-test.json | 28 ++++++++++++-------------
>  4 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index ced7a5ffe1..b154eae82e 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -859,9 +859,9 @@ longhand form of MEMBER.
>  Example: a struct type with unconditional member 'foo' and conditional
>  member 'bar' ::
>
> - { 'struct': 'IfStruct', 'data':
> -   { 'foo': 'int',
> -     'bar': { 'type': 'int', 'if': 'IFCOND'} } }
> + { 'struct': 'IfStruct',
> +   'data': { 'foo': 'int',
> +             'bar': { 'type': 'int', 'if': 'IFCOND'} } }
>
>  A union's discriminator may not be conditional.
>
> @@ -871,9 +871,9 @@ the longhand form of ENUM-VALUE_.
>  Example: an enum type with unconditional value 'foo' and conditional
>  value 'bar' ::
>
> - { 'enum': 'IfEnum', 'data':
> -   [ 'foo',
> -     { 'name' : 'bar', 'if': 'IFCOND' } ] }
> + { 'enum': 'IfEnum',
> +   'data': [ 'foo',
> +             { 'name' : 'bar', 'if': 'IFCOND' } ] }
>
>  Likewise, features can be conditional.  This requires the longhand
>  form of FEATURE_.
> diff --git a/tests/qapi-schema/doc-good.json
> b/tests/qapi-schema/doc-good.json
> index e0027e4cf6..cbf5c56c4b 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -60,8 +60,8 @@
>  #
>  # @two is undocumented
>  ##
> -{ 'enum': 'Enum', 'data':
> -  [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
> +{ 'enum': 'Enum',
> +  'data': [ { 'name': 'one', 'if': 'IFONE' }, 'two' ],
>    'features': [ 'enum-feat' ],
>    'if': 'IFCOND' }
>
> diff --git a/tests/qapi-schema/enum-if-invalid.json
> b/tests/qapi-schema/enum-if-invalid.json
> index 60bd0ef1d7..6bd20041f3 100644
> --- a/tests/qapi-schema/enum-if-invalid.json
> +++ b/tests/qapi-schema/enum-if-invalid.json
> @@ -1,3 +1,3 @@
>  # check invalid 'if' type
> -{ 'enum': 'TestIfEnum', 'data':
> -  [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
> +{ 'enum': 'TestIfEnum',
> +  'data': [ 'foo', { 'name' : 'bar', 'if': { 'val': 'foo' } } ] }
> diff --git a/tests/qapi-schema/qapi-schema-test.json
> b/tests/qapi-schema/qapi-schema-test.json
> index b6c36a9eee..3c43e14e22 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -220,27 +220,27 @@
>
>  # test 'if' condition handling
>
> -{ 'struct': 'TestIfStruct', 'data':
> -  { 'foo': 'int',
> -    'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
> +{ 'struct': 'TestIfStruct',
> +  'data': { 'foo': 'int',
> +            'bar': { 'type': 'int', 'if': 'TEST_IF_STRUCT_BAR'} },
>    'if': 'TEST_IF_STRUCT' }
>
> -{ 'enum': 'TestIfEnum', 'data':
> -  [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
> +{ 'enum': 'TestIfEnum',
> +  'data': [ 'foo', { 'name' : 'bar', 'if': 'TEST_IF_ENUM_BAR' } ],
>    'if': 'TEST_IF_ENUM' }
>
> -{ 'union': 'TestIfUnion', 'data':
> -  { 'foo': 'TestStruct',
> -    'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
> +{ 'union': 'TestIfUnion',
> +  'data': { 'foo': 'TestStruct',
> +            'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
>    'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
>
>  { 'command': 'test-if-union-cmd',
>    'data': { 'union-cmd-arg': 'TestIfUnion' },
>    'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
>
> -{ 'alternate': 'TestIfAlternate', 'data':
> -  { 'foo': 'int',
> -    'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
> +{ 'alternate': 'TestIfAlternate',
> +  'data': { 'foo': 'int',
> +            'bar': { 'type': 'TestStruct', 'if': 'TEST_IF_ALT_BAR'} },
>    'if': { 'all': ['TEST_IF_ALT', 'TEST_IF_STRUCT'] } }
>
>  { 'command': 'test-if-alternate-cmd',
> @@ -256,9 +256,9 @@
>
>  { 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
>
> -{ 'event': 'TEST_IF_EVENT', 'data':
> -  { 'foo': 'TestIfStruct',
> -    'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
> +{ 'event': 'TEST_IF_EVENT',
> +  'data': { 'foo': 'TestIfStruct',
> +            'bar': { 'type': ['TestIfEnum'], 'if': 'TEST_IF_EVT_BAR' } },
>    'if': { 'all': ['TEST_IF_EVT', 'TEST_IF_STRUCT'] } }
>
>  { 'event': 'TEST_IF_EVENT2', 'data': {},
> --
> 2.31.1
>
>
Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
Posted by Markus Armbruster 4 years, 4 months ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Break lines between members instead of within members.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> I wish we would just automated tools to format files. With that git 2.23
> feature, no more excuses :):
> https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame

Interesting.

> Sadly, our mixed json/py format doesn't get parsed after "blake" reformats
> it (strings or trailing commas).

Naming QAPI schema files .json even though their contents isn't was a
mistake.

We discussed possible improvements in the thread below

    Message-ID: <87ime52wxd.fsf@dusky.pond.sub.org>
    https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08325.html

Too much to read, but there's a summary:

    Message-ID: <877dt5ofoi.fsf@dusky.pond.sub.org>
    https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg02553.html

The least invasive way to achieve formatting automation could be
switching from bastardized JSON to proper subset of Python.

What's "blake"?


Re: [PATCH 01/22] qapi: Tidy up unusual line breaks
Posted by Marc-André Lureau 4 years, 4 months ago
Hi

On Mon, Sep 13, 2021 at 5:55 PM Markus Armbruster <armbru@redhat.com> wrote:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Hi
> >
> > On Mon, Sep 13, 2021 at 4:39 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Break lines between members instead of within members.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > I wish we would just automated tools to format files. With that git 2.23
> > feature, no more excuses :):
> >
> https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame
>
> Interesting.
>

Yeah, unfortunately you would have to add the commit rev after it's merged.


> > Sadly, our mixed json/py format doesn't get parsed after "blake"
> reformats
> > it (strings or trailing commas).
>
> Naming QAPI schema files .json even though their contents isn't was a
> mistake.
>
> We discussed possible improvements in the thread below
>
>     Message-ID: <87ime52wxd.fsf@dusky.pond.sub.org>
>     https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08325.html
>
> Too much to read, but there's a summary:
>
>     Message-ID: <877dt5ofoi.fsf@dusky.pond.sub.org>
>     https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg02553.html
>
> The least invasive way to achieve formatting automation could be
> switching from bastardized JSON to proper subset of Python.
>

ok


> What's "blake"?
>

Sorry, don't know why I always remember "blake" when it is simply  "black" (
https://github.com/psf/black)