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

✨ Add show_warning_types configuration variable #12131

Merged
merged 10 commits into from Mar 19, 2024

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Mar 18, 2024

This PR add the show_warning_types config variable which when set to True, prepends the type and subtype (if set) to warning messages.
For example:

WARNING: py:class reference target not found: TextElement [ref.class]

This follows the best practices of other tools, such as mypy and ruff to provide the user greater information on the warning:

  1. To provide greater specificity for the origin of the warning, particularly for extensions which utilise warning types. Currently these extensions have to provide bespoke solutions for showing the warning origin (see e.g. https://myst-parser.readthedocs.io/en/latest/configuration.html#build-warnings)

  2. To encourage users to deal with warnings. I strongly recommend for users to run sphinx-build -nW --keep-going, then fix all warnings. Practically this is not always possible and so the use of show_warning_types and suppress_warnings, provides a way for users to deal with most warnings, whilst being able to suppress "less important" ones

Note currently, the default is False, i.e. they will still not be shown, which is done in order to introduce any breaking changes.
But, like python/mypy#13542, I would strongly encourage this eventualy defaulting to True.

closes #8845

@picnixz
Copy link
Member

picnixz commented Mar 19, 2024

I remember there is a related issue or at least a comment about it Had seen it yesterday so I'll try to find it again.

sphinx/util/logging.py Outdated Show resolved Hide resolved
sphinx/util/logging.py Outdated Show resolved Hide resolved
sphinx/util/logging.py Outdated Show resolved Hide resolved
tests/test_util/test_util_logging.py Outdated Show resolved Hide resolved
chrisjsewell and others added 5 commits March 19, 2024 09:37
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
doc/usage/configuration.rst Outdated Show resolved Hide resolved
sphinx/config.py Outdated Show resolved Hide resolved
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@chrisjsewell chrisjsewell linked an issue Mar 19, 2024 that may be closed by this pull request
@chrisjsewell
Copy link
Member Author

I remember there is a related issue or at least a comment about it Had seen it yesterday so I'll try to find it again.

found it

@picnixz
Copy link
Member

picnixz commented Mar 19, 2024

LGTM!

@picnixz picnixz merged commit b0f096f into sphinx-doc:master Mar 19, 2024
22 checks passed
@chrisjsewell
Copy link
Member Author

Ah @picnixz its not the end of the world, but I would ask in future you don't merge my PRs 😬

I'm very "encouraging" of having good commit messages, and I was about to commit this with most of #12131 (comment) as the message

@picnixz
Copy link
Member

picnixz commented Mar 19, 2024

Ah @picnixz its not the end of the world, but I would ask in future you don't merge my PRs 😬

ah sorry! I just got the notification now for the ruff PR as well (since you replied with an emoji I thought it was fine)

@chrisjsewell
Copy link
Member Author

ah sorry! I just got the notification now for the ruff PR as well (since you replied with an emoji I thought it was fine)

No worries! As I mentioned, I just like to make sure they have a good commit message

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show warnings with their "type" and "subtype"?
2 participants