qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v5 11/36] qapi/common.py: Add indent manager


From: John Snow
Subject: Re: [PATCH v5 11/36] qapi/common.py: Add indent manager
Date: Thu, 8 Oct 2020 13:45:08 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 10/8/20 3:23 AM, Markus Armbruster wrote:
Eduardo Habkost <ehabkost@redhat.com> writes:

On Wed, Oct 07, 2020 at 02:08:33PM -0400, John Snow wrote:
On 10/7/20 4:50 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

Code style tools really dislike the use of global keywords, because it
generally involves re-binding the name at runtime which can have strange
effects depending on when and how that global name is referenced in
other modules.

Make a little indent level manager instead.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>

Intentation is a job for QAPIGen (and its subtypes).  But if this patch
is easier to achieve this series' goal, I don't mind.


I agree, but refactoring it properly is beyond my capacity right now.

This was the dumbest thing I could do to get pylint/mypy passing, which
required the elimination (or suppression) of the global keyword.

Creating a stateful object was the fastest way from A to B.

An even dumber solution could be:

   indent_prefixes = []
   def push_indent(amount=4):
       """Add `amount` spaces to indentation prefix"""
       indent_prefixes.push(' '*amount)
   def pop_indent():
       """Revert the most recent push_indent() call"""
       indent_prefixes.pop()
   def genindent():
       """Return the current indentation prefix"""
       return ''.join(indent_prefixes)

No global keyword involved, and the only stateful object is a
dumb list.

Ha, this is Dumb with a capital D!  I respect that :)

John, I'm not asking you to switch.  Use your judgement.


It's something we'll revisit soon, I'm sure. it's not good to be managing indent in so many different ways in so many different places.

(I prefer to leave it alone for now to try and press forward with accomplishing regression checks on strict typing.)

--js




reply via email to

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