Add more warning types and sub-types #11677
Conversation
Following sphinx-doc#3142, sphinx-doc#7460, sphinx-doc#9628, there are still some logger.warning in toc parsing and rendering (not in extensions) not categorized. This patch introduce DocumentError in environment adapter, reuse subtype `excluded`, `not_readable`, and add 4 `toc` subtypes, with the help of `grep`.
Following sphinx-doc#10107, sphinx-doc#11412(deprecated warn), add relevent type and subtype(`i18n` only) to logger.warning. Add warning in `util/i18n.py` to show the fallback locale. This commit with 7f5ab05 should fix ambiguous or dysfunctional configuration values of :confval:`suppress_warnings`.
Since the `application.py` has the warning of write or read ahead of "doing serial %s", it is safe to remove the second warning. Replace "should be considered experimental" with proposal typename `ext_*`. Conclude this patch along with 7f5ab05 and bdea05e in `versionchange::`. Correct the former bad typesetting. TODO: change the regex match of warning type, add test file to make `ext_*` type robust usage in `application.py`. TODO: community advice from PR.
Tiger3018
force-pushed
the
master
branch
3 times, most recently
from
d75e285 to
49eee15
Compare
doc/usage/configuration.rst
Outdated
| Make ``toc``, ``i18n`` and ``index`` warning types cover all relative | ||
| warnings. | ||
|
|
||
| Added ``i18n.babel``, ``i18n.not_readable``, ``i18n.not_writeable``, |
There was a problem hiding this comment.
What would those warnings mean?
Actually we should maybe have a follow-up issue which documents where and when warnings of such type/subtype are emitted because it's not always clear.
| logger.warning(message, ref, location=toctreenode) | ||
| raise | ||
| logger.warning(message, ref, location=toctreenode, type='toc', subtype=subtype) | ||
| raise DocumentError(message % ref) from err |
There was a problem hiding this comment.
Why this change? We should probably keep the KeyError because I have no idea if it is caught somewhere else differently
There was a problem hiding this comment.
Only in environment/init.py#L455 when there is an OSError.
title = ref and nodes.reference(..., *[nodes.Text(title)]) in following lines might be only KeyError source. I think this DocumentError could be outstanding for users to check original document status or filesystem status.
There was a problem hiding this comment.
For now, I would keep the exception type because they might catch it or assume its type. Changing the exception type could lead to incompatible changes and this requires a major version.
sphinx/util/nodes.py
Outdated
| logger.warning(__('toctree contains ref to nonexisting file %r'), | ||
| includefile, location=docname) | ||
| includefile, location=docname, | ||
| type='toc', subtype='not_readable') |
sphinx/util/i18n.py
Outdated
| return formatter(date, format, locale=locale) | ||
| except (ValueError, babel.core.UnknownLocaleError): | ||
| # fallback to English | ||
| logger.warning(__('Invalid locale for babel. Fallback locale to `en`.'), |
There was a problem hiding this comment.
Perhaps you could indicate the bad locale? because such warming is not necessarily helpful otherwise.
|
Hi, is this PR still in progress to be merged soon @Tiger3018 @picnixz? |
|
I personally am not involved in this PR and won't be taking over so it's up to the author to decide when it's ready. I'll nonetheless review the PR when it's updated. |
# Conflicts: # doc/usage/configuration.rst # sphinx/environment/adapters/toctree.py
AA-Turner
added
the
awaiting:response
# Conflicts: # doc/usage/configuration.rst # sphinx/directives/other.py # sphinx/environment/__init__.py # sphinx/environment/adapters/indexentries.py # sphinx/environment/adapters/toctree.py # sphinx/util/i18n.py # sphinx/util/nodes.py
AA-Turner
changed the title
suppress_warnings bugfix and refactoring towards extension
AA-Turner
added
internals:config
and removed
awaiting:response
4 participants
Subject:
Bugfix and Refactoring
Bugfix： bdea05e 7f5ab05
Refactoring(proposal): Add warning type and subtype for serial read warnings #11003 a07f439
Purpose
Please see commit message and its TODO.
Detail
(Refactoring) From commit message
Remove: Since the
application.pyhas the warning of write or read ahead of "doing serial %s", it is safe to remove the second warning.Documentation: Replace "should be considered experimental" with proposal typename
ext_*util/logging.py: Change the regex match of warning type.Test:
application.py:Relates