Skip to content

Add remove_old configuration to sphinx.ext.apidoc #13668

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

echoix
Copy link

@echoix echoix commented Jun 15, 2025

Makes the option available to be configured when using apidoc as an extension instead of as a command-line tool.

Purpose

The command-line version of apidoc allows to specify the --remove-old option. When passed, the value is true.
With the extension version of apidoc, sphinx.ext.apidoc, remove_old defaults to True, but isn't configurable.

suffix: str = 'rst'
remove_old: bool = True
quiet: bool = False

This means that when refactoring an old project to use the apidoc extension (in conf.py) instead of a customized Makefile with multiple handwritten calls to sphinx-apidoc, whilst wanting to keep the same generated output structure is impossible. The extension deletes all other files in the target directory (specified with 'destination''s value of apidoc_modules configuration option). It deletes all other handwritten files, and even deletes the index.rst, which causes other failures in the build process.

I understand that most people would want to generate these files in a subfolder of their source directory, but for legacy reasons, some contents and hand-written overrides are all in the same folder, and the pages name should be kept the same for our use-case.

Having the remove_old configuration value ported to the extension's config, even if not used by our project's makefile with the cli-based version, is the less invasive way for sphinx to allow our use case, and prevent the unexpected deletion of files when building (at least we have git to restore committed files). The unexpected part of the behaviour difference when migrating is caused by the default value that is different. It is False for the cli (opt-in), but True (non-configurable) when used as an extension.

With this PR, the config value added is then properly used at the very end of _run_apidoc_module(), that guards calling _remove_old_files() or not (line 89):

def _run_apidoc_module(
i: int,
*,
options: dict[str, Any],
defaults: ApidocDefaults,
srcdir: Path,
confdir: Path,
) -> None:
"""Run apidoc for a single module."""
args = _parse_module_options(
i, options=options, defaults=defaults, srcdir=srcdir, confdir=confdir
)
if args is None:
return
exclude_patterns_compiled: list[re.Pattern[str]] = [
re.compile(fnmatch.translate(exclude)) for exclude in args.exclude_pattern
]
written_files, modules = recurse_tree(
args.module_path, exclude_patterns_compiled, args, args.template_dir
)
if args.toc_file:
written_files.append(
create_modules_toc_file(modules, args, args.toc_file, args.template_dir)
)
if args.remove_old:
_remove_old_files(written_files, args.dest_dir, args.suffix)

Since I'm new to this code base (and never wrote an extension), I would request to take a special look at the very-end of main() in sphinx/ext/apidoc/_cli.py, to make sure the default value is still the same after this change. I'm still not sure, even after having applied the changes locally.

def main(argv: Sequence[str] = (), /) -> int:
"""Run the apidoc CLI."""
locale.setlocale(locale.LC_ALL, '')
sphinx.locale.init_console()
opts = _parse_args(argv)
rootpath = opts.module_path
excludes = tuple(
re.compile(fnmatch.translate(str(Path(exclude).resolve())))
for exclude in dict.fromkeys(opts.exclude_pattern)
)
written_files, modules = recurse_tree(rootpath, excludes, opts, opts.template_dir)
if opts.full:
_full_quickstart(opts, modules=modules)
elif opts.toc_file:
written_files.append(
create_modules_toc_file(modules, opts, opts.toc_file, opts.template_dir)
)
if opts.remove_old and not opts.dry_run:
_remove_old_files(written_files, opts.dest_dir, opts.suffix)
return 0

However, I ran tox on my repo of this fork once I applied the same changes for this PR, and the existing tests passed. There is one test that is found when searching for "remove_old" or "remove-old", but it didn't catch the behaviour change between the cli and extension usage, so...
def test_remove_old_files(tmp_path: Path):
"""Test that old files are removed when using the -r option.
Also ensure that pre-existing files are not re-written, if unchanged.
This is required to avoid unnecessary rebuilds.
"""
module_dir = tmp_path / 'module'
module_dir.mkdir()
(module_dir / 'example.py').touch()
gen_dir = tmp_path / 'gen'
gen_dir.mkdir()
(gen_dir / 'other.rst').touch()
apidoc_main(['-o', str(gen_dir), str(module_dir)])
assert set(gen_dir.iterdir()) == {
gen_dir / 'modules.rst',
gen_dir / 'example.rst',
gen_dir / 'other.rst',
}
example_mtime = (gen_dir / 'example.rst').stat().st_mtime_ns
apidoc_main(['--remove-old', '-o', str(gen_dir), str(module_dir)])
assert set(gen_dir.iterdir()) == {gen_dir / 'modules.rst', gen_dir / 'example.rst'}
assert (gen_dir / 'example.rst').stat().st_mtime_ns == example_mtime

References

  • None found

Note:
I will need to update the CHANGES.rst entry with the pull-request number once the PR is filed, and the PR number is assigned. Changed

echoix added 2 commits June 15, 2025 21:19
Makes the option available to be configured when using apidoc as an extension instead of as a command-line tool.
@echoix echoix force-pushed the apidoc-remove_old branch 2 times, most recently from 7097eac to 1504392 Compare June 15, 2025 22:11
@echoix echoix force-pushed the apidoc-remove_old branch from 1504392 to aac8a28 Compare June 15, 2025 22:13
@echoix echoix changed the title Add remove_old configuration to sphinx.ext.apidoc Add remove_old configuration to sphinx.ext.apidoc Jun 16, 2025
@echoix echoix changed the title Add remove_old configuration to sphinx.ext.apidoc Add remove_old configuration to sphinx.ext.apidoc Jun 16, 2025
@echoix
Copy link
Author

echoix commented Jun 23, 2025

Am I missing something before the first contact?

@echoix
Copy link
Author

echoix commented Jun 24, 2025

Solved the conflicts while awaiting the first review

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

Successfully merging this pull request may close these issues.

2 participants