[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: John Snow
Subject: Re: [PATCH 06/37] qapi: delint using flake8
Date: Fri, 18 Sep 2020 14:13:58 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 9/18/20 6:33 AM, Markus Armbruster wrote:
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.
       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
several complaints to ignore, so I left the code alone.
If it gets in the way of running them automatically, and messing up
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

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.

Your wish is my command!
(Except in the other replies where I am arguing with you.)

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.

Bigger fish to fry, but with other languages like rust looming, making the core generation facilities nicer might be ...nice. Not for this series.

Two approaches in general would make sense to me:

1. Building an AST for C instead of strings, and rendering the AST.
2. Writing a templating engine that doesn't break the Python indentation flow by hoisting them into module constants and improving the rendering logic.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]