[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 05/17] qapi: pass QAPISchemaModule to visit_module instead
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 05/17] qapi: pass QAPISchemaModule to visit_module instead of str |
Date: |
Thu, 21 Jan 2021 08:22:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> On 1/20/21 7:07 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
[...]
>>> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
>>> index e03abcbb959..f754f675d66 100644
>>> --- a/docs/sphinx/qapidoc.py
>>> +++ b/docs/sphinx/qapidoc.py
>>> @@ -463,11 +463,11 @@ def __init__(self, env, qapidir):
>>> self._env = env
>>> self._qapidir = qapidir
>>> - def visit_module(self, name):
>>> - if name is not None:
>>> - qapifile = self._qapidir + '/' + name
>>> + def visit_module(self, module):
>>> + if module.name:
>> Replacing the "is not None" test by (implicit) "is thruthy" changes
>> behavior for the empty string. Intentional?
>>
>
> Instinctively it was intentional, consciously it wasn't. I was worried
> about what "qapifile" would produce if the string happened to be
> empty.
It would produce a dependency on the directory.
>> I've had the "pleasure" of debugging empty strings getting interpreted
>> like None where they should be interpreted like any other string.
>>
>
> assert module.name, then?
Let's avoid changing behavior in a refactoring patch. If you want an
assertion to ease your worry, separate patch.
>>> + qapifile = self._qapidir + '/' + module.name
>>> self._env.note_dependency(os.path.abspath(qapifile))
>>> - super().visit_module(name)
>>> + super().visit_module(module)
>>>
>> [...]
>>
- [PATCH v3 00/17] qapi: static typing conversion, pt1.5, John Snow, 2021/01/19
- [PATCH v3 03/17] qapi/main: handle theoretical None-return from re.match(), John Snow, 2021/01/19
- [PATCH v3 01/17] qapi/commands: assert arg_type is not None, John Snow, 2021/01/19
- [PATCH v3 06/17] qapi: centralize is_[user|system|builtin]_module methods, John Snow, 2021/01/19
- [PATCH v3 08/17] qapi: use explicitly internal module names, John Snow, 2021/01/19
- [PATCH v3 05/17] qapi: pass QAPISchemaModule to visit_module instead of str, John Snow, 2021/01/19
[PATCH v3 07/17] qapi/gen: Replace ._begin_system_module(), John Snow, 2021/01/19
[PATCH v3 11/17] qapi: centralize the built-in module name definition, John Snow, 2021/01/19
[PATCH v3 04/17] qapi/gen: inline _wrap_ifcond into end_if(), John Snow, 2021/01/19
[PATCH v3 12/17] qapi/gen: write _genc/_genh access shims, John Snow, 2021/01/19
[PATCH v3 16/17] qapi: type 'info' as Optional[QAPISourceInfo], John Snow, 2021/01/19
[PATCH v3 10/17] qapi/gen: Combine ._add_[user|system]_module, John Snow, 2021/01/19