[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 11/36] qapi/common.py: Add indent manager
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 11/36] qapi/common.py: Add indent manager |
Date: |
Thu, 08 Oct 2020 09:23:14 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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.
- Re: [PATCH v5 06/36] qapi: Remove wildcard includes, (continued)
[PATCH v5 12/36] qapi/common.py: delint with pylint, John Snow, 2020/10/05
[PATCH v5 13/36] qapi/common.py: Replace one-letter 'c' variable, John Snow, 2020/10/05
[PATCH v5 14/36] qapi/common.py: check with pylint, John Snow, 2020/10/05
[PATCH v5 15/36] qapi/common.py: add type hint annotations, John Snow, 2020/10/05
[PATCH v5 16/36] qapi/common.py: Convert comments into docstrings, and elaborate, John Snow, 2020/10/05