Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostics v3 #99

Merged
merged 4 commits into from
Dec 7, 2022
Merged

Diagnostics v3 #99

merged 4 commits into from
Dec 7, 2022

Conversation

jnikula
Copy link
Owner

@jnikula jnikula commented Dec 7, 2022

Third time's the charm? Superseeds #98.

Wrap parser diagnostics in a simple dataclass, with a helper for
formatting the message.
Properly propagate error messages from _recursive_parse() all the way to
Sphinx logging to improve diagnostics on comments above unexpected
cursors.

Also preserve the comments and output them as regular top level comments
instead of just dropping them and replacing with diagnostics in the
produced documentation.

Fixes: #80
Extend the ErrorLevel enum to cover all Clang diagnostic levels. Use the
Clang diagnostic levels as the values to avoid mapping in the parser,
but this is an implementation detail. Make it an IntEnum to ensure the
values are indeed ints.

Map the ErrorLevel values to Sphinx verbosity levels and log level names
in the extension, and avoid making assumptions about the values.

The verbosity levels are a bit weird, but for now keep them functionally
equivalent to what we had before. We may want to revisit them later.

Assume the log level names we specify here are stable in Sphinx, and
have Sphinx map them to its internal log level values within log().
@jnikula jnikula requested a review from BrunoMSantos December 7, 2022 16:58
Copy link
Collaborator

@BrunoMSantos BrunoMSantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for sticking with me :)

Just double check the last commit message. It states:

Start logging parser warnings at default verbosity level 0, parser info at verbosity level 1, and parser debug at verbosity level 1.

It mentions level 1 twice, whereas I believe you mean 2 for debug and verbosity levels.

Instead of having our own verbosity filtering, map parser diagnostics to
level names that have the verbosity we want, and let Sphinx logging do
the filtering behind the scenes.

Start logging parser warnings at default verbosity level 0, parser info
at verbosity level 1, and parser debug at verbosity level 2.
@jnikula
Copy link
Owner Author

jnikula commented Dec 7, 2022

Nice! Thanks for sticking with me :)

Likewise! :)

I really appreciate the review. Thanks!

Just double check the last commit message. It states:

Start logging parser warnings at default verbosity level 0, parser info at verbosity level 1, and parser debug at verbosity level 1.

It mentions level 1 twice, whereas I believe you mean 2 for debug and verbosity levels.

D'oh! Fixed, thanks.

@jnikula jnikula merged commit 342e89f into master Dec 7, 2022
@jnikula jnikula deleted the diagnostics-v3 branch December 7, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants