Re: [PATCH v3 4/9] qapi: introduce IfPredicateList and IfAny

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2816 bytes --]

Hi

On Thu, May 13, 2021 at 3:26 AM John Snow <jsnow@redhat.com> wrote:

> On 4/29/21 9:40 AM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Refactor IfAll class, to introduce a base class IfPredicateList and add
> > IfAny for the 'any' conditions.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   scripts/qapi/common.py | 32 +++++++++++++++++++++++++++-----
> >   1 file changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 59a7ee2f32..102d347348 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -224,23 +224,45 @@ def __eq__(self, other: object) -> bool:
> >           return self.option == other.option
> >
> >
> > -class IfAll(IfPredicate):
> > +class IfPredicateList(IfPredicate):
> > +    C_SEP = ""
> > +    DOC_SEP = ""
> > +
> >       def __init__(self, pred_list: Sequence[IfPredicate]):
> >           self.pred_list = pred_list
> >
> >       def cgen(self) -> str:
> > -        return " && ".join([p.cgen() for p in self.pred_list])
> > +        sep = " " + self.C_SEP + " "
> > +        gen = sep.join([p.cgen() for p in self.pred_list])
> > +        if len(self.pred_list) == 1:
> > +            return gen
> > +        return "(%s)" % gen
> >
> >       def docgen(self) -> str:
> > -        return " and ".join([p.docgen() for p in self.pred_list])
> > +        sep = " " + self.DOC_SEP + " "
> > +        gen = sep.join([p.docgen() for p in self.pred_list])
> > +        if len(self.pred_list) == 1:
> > +            return gen
> > +        return "(%s)" % gen
> >
> >       def __bool__(self) -> bool:
> >           return bool(self.pred_list)
> >
> >       def __repr__(self) -> str:
> > -        return f"IfAll({self.pred_list})"
> > +        ty = type(self)
> > +        return f"{ty.__qualname__}({self.pred_list})"
> >
> >       def __eq__(self, other: object) -> bool:
> > -        if not isinstance(other, IfAll):
> > +        if not isinstance(other, type(self)):
> >               return False
> >           return self.pred_list == other.pred_list
> > +
> > +
> > +class IfAll(IfPredicateList):
> > +    C_SEP = "&&"
> > +    DOC_SEP = "and"
> > +
> > +
> > +class IfAny(IfPredicateList):
> > +    C_SEP = "||"
> > +    DOC_SEP = "or"
> >
>
> I do like the way these get combined. Is there a reason it's not
> squashed into the earlier commit?
>
>
No, just the sake of doing things iteratively.

(Qualms about not having a visitor or a callback or whatever you want to
> call it remain, but I'll stop remarking on that for the rest of this
> series.)
>
>
thanks :)

Tested-by: John Snow <jsnow@redhat.com>
>
>

[-- Attachment #2: Type: text/html, Size: 4487 bytes --]