Re: [PATCH v3] qapi: provide a friendly string representation of QAPI classes

[Daniel P . Berrangé]

On Wed, Oct 18, 2023 at 02:37:45PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > If printing a QAPI schema object for debugging we get the classname and
> > a hex value for the instance:
> >
> >   <qapi.schema.QAPISchemaEnumType object at 0x7f0ab4c2dad0>
> >   <qapi.schema.QAPISchemaObjectType object at 0x7f0ab4c2dd90>
> >   <qapi.schema.QAPISchemaArrayType object at 0x7f0ab4c2df90>
> >
> > With this change we instead get the classname and the human friendly
> > name of the QAPI type instance:
> >
> >   <QAPISchemaEnumType:CpuS390State at 0x7f0ab4c2dad0>
> >   <QAPISchemaObjectType:CpuInfoS390 at 0x7f0ab4c2dd90>
> >   <QAPISchemaArrayType:CpuInfoFastList at 0x7f0ab4c2df90>
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >
> > Changed in v3:
> >
> >  - Retain the object hex ID in the new representation
> >
> >  scripts/qapi/schema.py | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index 231ebf61ba..39c11bb52a 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -73,6 +73,12 @@ def __init__(self, name: str, info, doc, ifcond=None, 
> > features=None):
> >          self.features = features or []
> >          self._checked = False
> >  
> > +    def __repr__(self):
> > +        if self.name is not None:
> > +            return "<%s:%s at 0x%x>" % (type(self).__name__, self.name, 
> > id(self))
> > +        else:
> > +            return "<%s at 0x%x>" % (type(self).__name__, id(self))
> > +
> >      def c_name(self):
> >          return c_name(self.name)
> 
> Looks good now.
> 
> Except I generally prefer
> 
>     if COND:
>         SUITE1
>     else:
>         SUITE2
> 
> over
> 
>     if not COND:
>         SUITE2
>     else:
>         SUITE1
> 
> because it avoids mental double-negation.
>     
> Mind if I swap things?  Like so:
> 
>     def __repr__(self):
>         if self.name is None:
>             return "<%s at 0x%x>" % (type(self).__name__, id(self))
>         else:
>             return "<%s:%s at 0x%x>" % (type(self).__name__,
>                                         self.name, id(self))

Sure, fine with me.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|