Re: [PATCH 06/37] qapi: delint using flake8

From: Markus Armbruster <armbru@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	qemu-devel@nongnu.org, "Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH 06/37] qapi: delint using flake8
Date: Fri, 18 Sep 2020 12:33:17 +0200	[thread overview]
Message-ID: <87bli3jrwy.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <82b4b8ce-b348-75fd-bd44-d96303a4875d@redhat.com> (John Snow's message of "Thu, 17 Sep 2020 12:57:02 -0400")

John Snow <jsnow@redhat.com> writes:

> On 9/17/20 3:54 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> On 9/16/20 8:12 AM, Markus Armbruster wrote:
>>>> John Snow <jsnow@redhat.com> writes:
>>>>
>>>>> Petty style guide fixes and line length enforcement.  Not a big win, not
>>>>> a big loss, but flake8 passes 100% on the qapi module, which gives us an
>>>>> easy baseline to enforce hereafter.
>>>>>
>>>>> Signed-off-by: John Snow <jsnow@redhat.com>
[...]
>>>>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
>>>>> index e1df0e341f..2e4b4de0fa 100644
>>>>> --- a/scripts/qapi/commands.py
>>>>> +++ b/scripts/qapi/commands.py
>>>>> @@ -69,7 +69,8 @@ def gen_call(name, arg_type, boxed, ret_type):
>>>>>    def gen_marshal_output(ret_type):
>>>>>        return mcgen('''
>>>>>    -static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
>>>>> QObject **ret_out, Error **errp)
>>>>> +static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
>>>>> +                                          Error **errp)
>>>> The continued parameter list may become misalignd in generated C.
>>>> E.g.
>>>>       static void qmp_marshal_output_BlockInfoList(BlockInfoList *ret_in, QObject **ret_out,
>>>>                                                 Error **errp)
>>>>       {
>>>>       ...
>>>>       }
>>>> Do we care?
>>>>
>>>
>>> Yeah, I don't know. Do we?
>> I care, but I also care for automated style checks.
>> 
>>> It actually seemed more annoying to try and get flake8 to make an
>>> exception for these handful of examples.
>>>
>>> Path of least resistance led me here, but I can try and appease both
>>> systems if you'd prefer.
>> Up to now, I ran the style checkers manually, and this was just one
>> of
>> several complaints to ignore, so I left the code alone.
>> If it gets in the way of running them automatically, and messing up
>> the
>> generated code slightly is the easiest way to get it out of the way,
>> then I can accept the slight mess.
>> 
>
> I changed this a little to put all the args on the next line, which is
> slightly unusual but works okay.

I think it's slightly more unusual than the non-matching indentation
was.

Yet another way to skin this cat:

    static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
                                    QObject **ret_out, Error **errp)

Now the second line is not aligned with the left parenthesis both in the
Python source and in the generated C.

> I think that's a fine middle ground, because the alternative (to me)
> is to start using abstracted code generation tokens in a tree
> structure, etc etc etc.
>
> Embedded templates are always gonna look kinda nasty, I think, because
> you're trying to fight style guidelines in two languages
> simultaneously and it's never gonna quite work out exactly how you
> want without some pretty complex abstraction mechanisms that are well
> beyond the power we need right now.

The thought "screw this, pile the output through /usr/bin/indent" has
crossed my mind more than once.