| Message ID | 20240514215740.940155-7-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: new sphinx qapi domain pre-requisites | expand |
John Snow <jsnow@redhat.com> writes: > If a comment immediately follows a doc block, the parser doesn't ignore > that token appropriately. Fix that. Reproducer? > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/parser.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index 41b9319e5cb..161768b8b96 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc': > line = self.get_doc_line() > first = False > > - self.accept(False) > + self.accept() > doc.end() > return doc Can't judge the fix without understanding the problem, and the problem will be easier to understand for me with a reproducer.
On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > If a comment immediately follows a doc block, the parser doesn't ignore > > that token appropriately. Fix that. > > Reproducer? > > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > scripts/qapi/parser.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > > index 41b9319e5cb..161768b8b96 100644 > > --- a/scripts/qapi/parser.py > > +++ b/scripts/qapi/parser.py > > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc': > > line = self.get_doc_line() > > first = False > > > > - self.accept(False) > > + self.accept() > > doc.end() > > return doc > > Can't judge the fix without understanding the problem, and the problem > will be easier to understand for me with a reproducer. > audio.json: ``` ## # = Audio ## ## # @AudiodevPerDirectionOptions: # # General audio backend options that are used for both playback and # recording. # ``` Modify this excerpt to have a comment after the "= Audio" header, say for instance if you were to take out that intro paragraph and transform it into a comment that preceded the AudiodevPerDirectionOptions doc block. e.g. ``` ## # = Audio ## # Lorem Ipsum ## # @AudiodevPerDirectionOptions: ``` the parser breaks because the line I changed that primes the next token is still set to "not ignore comments", but that breaks the parser and gives a rather unhelpful message: ../qapi/audio.json:13:1: junk after '##' at start of documentation comment Encountered when converting developer commentary from documentation paragraphs to mere QAPI comments. --js
John Snow <jsnow@redhat.com> writes: > On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com> wrote: > >> John Snow <jsnow@redhat.com> writes: >> >> > If a comment immediately follows a doc block, the parser doesn't ignore >> > that token appropriately. Fix that. >> >> Reproducer? >> >> > >> > Signed-off-by: John Snow <jsnow@redhat.com> >> > --- >> > scripts/qapi/parser.py | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py >> > index 41b9319e5cb..161768b8b96 100644 >> > --- a/scripts/qapi/parser.py >> > +++ b/scripts/qapi/parser.py >> > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc': >> > line = self.get_doc_line() >> > first = False >> > >> > - self.accept(False) >> > + self.accept() >> > doc.end() >> > return doc >> >> Can't judge the fix without understanding the problem, and the problem >> will be easier to understand for me with a reproducer. >> > > audio.json: > > ``` > ## > # = Audio > ## > > ## > # @AudiodevPerDirectionOptions: > # > # General audio backend options that are used for both playback and > # recording. > # > ``` > > Modify this excerpt to have a comment after the "= Audio" header, say for > instance if you were to take out that intro paragraph and transform it into > a comment that preceded the AudiodevPerDirectionOptions doc block. > > e.g. > > ``` > ## > # = Audio > ## > > # Lorem Ipsum > > ## > # @AudiodevPerDirectionOptions: > ``` > > the parser breaks because the line I changed that primes the next token is > still set to "not ignore comments", but that breaks the parser and gives a > rather unhelpful message: > > ../qapi/audio.json:13:1: junk after '##' at start of documentation comment When ._parse()'s loop sees a comment token in .tok, it expects it to be the start of a doc comment block (because any other comments are to be skipped). It then calls .get_doc(), which expects the same. The unexpected comment token then triggers .get_doc()'s check for junk after '##'. > Encountered when converting developer commentary from documentation > paragraphs to mere QAPI comments. Your fix is correct. It's actually a regression. Please add Fixes: 3d035cd2cca6 (qapi: Rewrite doc comment parser) to the commit message. Let's add a reproducer to tests/qapi-schema/doc-good.json right in this patch, e.g. diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index de38a386e8..8b39eb946a 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -55,6 +55,8 @@ # - {braces} ## +# Not a doc comment + ## # @Enum: #
On Thu, Jun 13, 2024 at 10:32 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com> > wrote: > > > >> John Snow <jsnow@redhat.com> writes: > >> > >> > If a comment immediately follows a doc block, the parser doesn't > ignore > >> > that token appropriately. Fix that. > >> > >> Reproducer? > >> > >> > > >> > Signed-off-by: John Snow <jsnow@redhat.com> > >> > --- > >> > scripts/qapi/parser.py | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > >> > index 41b9319e5cb..161768b8b96 100644 > >> > --- a/scripts/qapi/parser.py > >> > +++ b/scripts/qapi/parser.py > >> > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc': > >> > line = self.get_doc_line() > >> > first = False > >> > > >> > - self.accept(False) > >> > + self.accept() > >> > doc.end() > >> > return doc > >> > >> Can't judge the fix without understanding the problem, and the problem > >> will be easier to understand for me with a reproducer. > >> > > > > audio.json: > > > > ``` > > ## > > # = Audio > > ## > > > > ## > > # @AudiodevPerDirectionOptions: > > # > > # General audio backend options that are used for both playback and > > # recording. > > # > > ``` > > > > Modify this excerpt to have a comment after the "= Audio" header, say for > > instance if you were to take out that intro paragraph and transform it > into > > a comment that preceded the AudiodevPerDirectionOptions doc block. > > > > e.g. > > > > ``` > > ## > > # = Audio > > ## > > > > # Lorem Ipsum > > > > ## > > # @AudiodevPerDirectionOptions: > > ``` > > > > the parser breaks because the line I changed that primes the next token > is > > still set to "not ignore comments", but that breaks the parser and gives > a > > rather unhelpful message: > > > > ../qapi/audio.json:13:1: junk after '##' at start of documentation > comment > > When ._parse()'s loop sees a comment token in .tok, it expects it to be > the start of a doc comment block (because any other comments are to be > skipped). It then calls .get_doc(), which expects the same. The > unexpected comment token then triggers .get_doc()'s check for junk after > '##'. > > > Encountered when converting developer commentary from documentation > > paragraphs to mere QAPI comments. > > Your fix is correct. > > It's actually a regression. Please add > > Fixes: 3d035cd2cca6 (qapi: Rewrite doc comment parser) > > to the commit message. > > Let's add a reproducer to tests/qapi-schema/doc-good.json right in this > patch, e.g. > > diff --git a/tests/qapi-schema/doc-good.json > b/tests/qapi-schema/doc-good.json > index de38a386e8..8b39eb946a 100644 > --- a/tests/qapi-schema/doc-good.json > +++ b/tests/qapi-schema/doc-good.json > @@ -55,6 +55,8 @@ > # - {braces} > ## > > +# Not a doc comment > + > ## > # @Enum: > # > Okey-dokey, all set. --js
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 41b9319e5cb..161768b8b96 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc': line = self.get_doc_line() first = False - self.accept(False) + self.accept() doc.end() return doc
If a comment immediately follows a doc block, the parser doesn't ignore that token appropriately. Fix that. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)