Re: [Qemu-devel] [PATCH v5 12/17] qapi: rename QAPIExprError/QAPILineError

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <mlureau@redhat.com> writes:

> ----- Original Message -----
>> Make the summary line "qapi: Rename QAPIExprError to QAPILineError".
>> 
>
> ok
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> 
>> > There is nothing specific about expressions in this exception,
>> > the following patch will use it without expressions.
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  scripts/qapi.py | 146
>> >  ++++++++++++++++++++++++++++----------------------------
>> >  1 file changed, 73 insertions(+), 73 deletions(-)
>> >
>> > diff --git a/scripts/qapi.py b/scripts/qapi.py
>> > index 21bc32f..4d1b0e4 100644
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -110,11 +110,11 @@ class QAPISchemaError(Exception):
>> >              "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
>> >  
>> >  
>> > -class QAPIExprError(Exception):
>> > -    def __init__(self, expr_info, msg):
>> > +class QAPILineError(Exception):
>> > +    def __init__(self, info, msg):
>> >          Exception.__init__(self)
>> > -        assert expr_info
>> > -        self.info = expr_info
>> > +        assert info
>> > +        self.info = info
>> >          self.msg = msg
>> >  
>> >      def __str__(self):
>> 
>> Since we're talking about misnamed / awkward error stuff:
>> 
>> * QAPISchemaError is really a parse error.  __init__()'s schema argument
>>   isn't a QAPISchema, it's a QAPISchemaParser.
>> 
>> * Method __str__() is mostly duplicated.
>> 
>> How do you like the following untested sketch?
>
> I like it, though I would consider it separately from this series.

Punting ideas we both like to later series is fair.  But when we intend
to rework the error classes later, renaming one of them now is perhaps
not worth the churn.

> Here I systematically renamed the existing class ("line" since it's
> about location in file), your proposed change has more implications I
> don't really want to get into here.

Well, QAPISchemaError is just as much about "location in file" as
QAPIExprError.  They just specify the location differently: one by
reference to the parser's current location, and the other by an "info"
dictionary.

[...]