On Fri, Sep 25, 2020 at 03:00:51PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabk...@redhat.com> writes:
>
> > On Tue, Sep 22, 2020 at 05:00:47PM -0400, John Snow wrote:
> >> The edge case is that if the name is '', this expression returns a
> >> string instead of a bool, which violates our declared type.
> >>
> >> Signed-off-by: John Snow <js...@redhat.com>
> >> ---
> >> scripts/qapi/gen.py | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> >> index 9898d513ae..cb2b2655c3 100644
> >> --- a/scripts/qapi/gen.py
> >> +++ b/scripts/qapi/gen.py
> >> @@ -251,7 +251,7 @@ def __init__(self, prefix, what, user_blurb,
> >> builtin_blurb, pydoc):
> >>
> >> @staticmethod
> >> def _is_user_module(name):
> >> - return name and not name.startswith('./')
> >> + return name is not None and not name.startswith('./')
> >
> > This changes behavior if name=='', and I guess this is OK, but
> > I'm not sure.
>
> @name is either
>
> (1) A module pathname relative to the main module
>
> This is a module defined by the user.
>
> (2) system module name, starting with './'
>
> This is a named system module. We currently have two: './init' in
> commands.py, and and './emit' in events.py.
>
> (3) None
>
> This is the (nameless) system module for built-in stuff. It
> predates (2). Using './builtin' would probably be better now.
>
> Note that (1) and (2) are disjoint: relative pathnames do not begin with
> './'.
>
> name='' is not possible, because '' is not a valid pathname.
Thanks! So, the './' prefix is just internal state and never
visible to the outside, correct? I would use a separate bool
instead of trying to encode additional state inside the string.
>
> > I miss documentation on `visit_module()`,
> > `visit_include()`, and `_is_user_module()`. I don't know what
> > `name` means here, and what is a "user module".
>
> Valid complaints! The code is subtle in places, without helping its
> readers along with comments or doc strings.
>
> >
> >>
> >> @staticmethod
> >> def _is_builtin_module(name):
> >> --
> >> 2.26.2
> >>
--
Eduardo
