♻️ refactor(logic): improve error handling and reduce duplication#286
Merged
gaborbernat merged 2 commits intotox-dev:mainfrom Mar 2, 2026
Merged
♻️ refactor(logic): improve error handling and reduce duplication#286gaborbernat merged 2 commits intotox-dev:mainfrom
gaborbernat merged 2 commits intotox-dev:mainfrom
Conversation
The dynamic import of user-specified modules produced raw Python tracebacks (ImportError, AttributeError) when users misconfigured :module: or :func: directive options. Wrapping these with explicit error messages surfaces the problem clearly in Sphinx's warning output. The module cleanup on the AttributeError path also prevents stale sys.modules entries from causing cross-test contamination when the same generic module name is reused. The duplicated prefix resolution logic between _build_opt_grp_title and _build_sub_cmd_title was fragile to maintain independently. Extracting _resolve_prefix consolidates the branching into one place. Coverage threshold was at 76% while actual coverage sat at ~99.7%, allowing significant regressions to pass unnoticed. Raised to 100% and added tests for previously uncovered argparse features (nargs, choices, count/append actions) and error paths.
After consolidating the ANSI stripping into _mk_sub_command's version guard, the stripped parameter in _build_sub_cmd_title became dead code — its only caller always passed stripped=True. This left _strip_ansi_colors unreachable on Python <3.14, failing coverage on those versions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When users misconfigure the
:module:or:func:directive options, the extension produced raw Python tracebacks (ImportError,AttributeError) that gave no indication of what went wrong or how to fix it. 🔐 This is especially confusing for new users who may not realize the error originates from their RST configuration rather than a bug in the extension.The dynamic import is now wrapped with explicit error handling that surfaces clear messages through Sphinx's warning system, e.g.
Failed to import module 'foo'orModule 'foo' has no attribute 'bar'. A subtlesys.modulesleak on theAttributeErrorpath was also fixed — when the module imported successfully but the function wasn't found, the stale module entry remained cached, which could cause the wrong parser to load in subsequent directive invocations within the same process.♻️ The duplicated prefix resolution logic between
_build_opt_grp_titleand_build_sub_cmd_titlehas been consolidated into a shared_resolve_prefixmethod. Both methods had near-identical branching for combiningtitle_prefix,sub_title_prefix, and program name, but diverged in small ways that made independent maintenance error-prone. A redundant_strip_ansi_colorscall was also eliminated —_mk_sub_commandalready strips ANSI fromparser.progbefore passing it to_build_sub_cmd_title, which was stripping it again.The coverage threshold has been raised from 76% to 100% to match actual coverage and prevent silent regressions. Additional test fixtures exercise previously untested argparse features (
nargsvariants,choices,action="count",action="append",required=True) and both error paths for bad module/function references.