[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/22] qapi/source: Remove line number from QAPISourceInfo in
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 03/22] qapi/source: Remove line number from QAPISourceInfo initializer |
Date: |
Sat, 24 Apr 2021 08:38:09 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> With the QAPISourceInfo(None, None, None) construct gone, there's not
> really any reason to have to specify that a file starts on the first
> line.
>
> Remove it from the initializer and have it default to 1.
>
> Remove the last vestiges where we check for 'line' being unset. That
> won't happen again, now!
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/parser.py | 2 +-
> scripts/qapi/source.py | 12 +++---------
> 2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index b378fa33807..edd0af33ae0 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -47,7 +47,7 @@ def __init__(self, fname, previously_included=None,
> incl_info=None):
> if self.src == '' or self.src[-1] != '\n':
> self.src += '\n'
> self.cursor = 0
> - self.info = QAPISourceInfo(fname, 1, incl_info)
> + self.info = QAPISourceInfo(fname, incl_info)
> self.line_pos = 0
> self.exprs = []
> self.docs = []
> diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
> index 21090b9fe78..afa21518974 100644
> --- a/scripts/qapi/source.py
> +++ b/scripts/qapi/source.py
> @@ -37,10 +37,9 @@ def __init__(self) -> None:
> class QAPISourceInfo:
> T = TypeVar('T', bound='QAPISourceInfo')
>
> - def __init__(self, fname: str, line: int,
> - parent: Optional['QAPISourceInfo']):
> + def __init__(self, fname: str, parent: Optional['QAPISourceInfo'] =
> None):
Not mentioned in the commit message: you add a default parameter value.
It's not used; there's just one caller, and it passes a value.
Intentional?
> self.fname = fname
> - self.line = line
> + self.line = 1
> self._column: Optional[int] = None
> self.parent = parent
> self.pragma: QAPISchemaPragma = (
> @@ -59,12 +58,7 @@ def next_line(self: T) -> T:
> return info
>
> def loc(self) -> str:
> - # column cannot be provided meaningfully when line is absent.
> - assert self.line or self._column is None
> -
> - ret = self.fname
> - if self.line is not None:
> - ret += ':%d' % self.line
> + ret = f"{self.fname}:{self.line}"
> if self._column is not None:
> ret += ':%d' % self._column
> return ret
Mixing f-string and % interpolation. I doubt we'd write it this way
from scratch. I recommend to either stick to % for now (leave
conversion to f-strings for later), or conver the column formatting,
too, even though it's not related to the patch's purpose.
[PATCH 04/22] qapi/parser: factor parsing routine into method, John Snow, 2021/04/21