Fixed #36485 -- Added linting tool for documentation files. #19549
Conversation
There was a problem hiding this comment.
Thank you @smithdc1 to invest time in this work. I think we are in the right direction! 🌟
I was wondering if you consider making this an extension: we would still need some customization/wrangling, but we would avoid all the new Makefile rule/tox changes/etc etc.
I gave this a try and have this MVP (still needs the customizations). In an ideal scenario, this would replace commits 1 and 6:
commit 48075d9be469154fb26abde37b955a39c2785b4a (HEAD -> pr/19549)
Author: Natalia <124304+nessita@users.noreply.github.com>
Date: Thu Jun 12 14:36:25 2025 -0300
Alternative to explicit `lint` Makefile rule via Sphinx Extensions.
diff --git a/docs/_ext/djangolint.py b/docs/_ext/djangolint.py
new file mode 100644
index 0000000000..4009dc5524
--- /dev/null
+++ b/docs/_ext/djangolint.py
@@ -0,0 +1,28 @@
+from sphinx.util import logging
+from sphinxlint.checkers import all_checkers
+from sphinxlint.sphinxlint import check_text
+
+logger = logging.getLogger(__name__)
+
+
+def lint_file(app, docname, source):
+ path = app.env.doc2path(docname)
+ content = source[0]
+
+ checkers = list(all_checkers.values())
+ for checker in checkers:
+ if ".rst" in checker.suffixes:
+ checker.suffixes = (*checker.suffixes, ".txt")
+
+ errors = check_text(path, content, checkers)
+ for error in errors:
+ logger.error(error)
+
+
+def setup(app):
+ app.connect("source-read", lint_file)
+ return {
+ "version": "1.0",
+ "parallel_read_safe": True,
+ "parallel_write_safe": True,
+ }
diff --git a/docs/conf.py b/docs/conf.py
index be79b9133c..1836eed3df 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -44,6 +44,7 @@ needs_sphinx = "4.5.0"
# coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
extensions = [
"djangodocs",
+ "djangolint",
"sphinx.ext.extlinks",
"sphinx.ext.intersphinx",
"sphinx.ext.autosectionlabel",|
For long lines that are links, there is an opened PR against the lint project: sphinx-contrib/sphinx-lint#132 |
I'm not sure. This would be a different approach taken with both
|
Thank you for this reasoning, these are all very valid points. My rationale was along these lines:
Re: |
|
Thank you Natalia. One other thing I thought about is having it available as a pre-commit hook. It seems that a separate repository would be required though and that feels a bit much. I think I'd rather try and discuss these changes with the sphinx-lint team and see if I can contribute the changes back upstream. This is a longer term goal though and shouldn't stop this work here. Now that we've had a good discussion on the pro/cons of some different approaches I'm happy to help implement this change in the way that you recommend. |
My recommendation would be to push forward the approach that you think it's going to be less work for you, while providing useful feedback to the contributors. I sense that you may prefer the separated, non extension, lint rule and I'm happy if we go that path (as you said, we have covered on pro/cons and there is no clear "winner"). Perhaps a good next step for a small, self contained commit would be to add a new Makefile rule "check" that would do black, lint and spelling at once? |
^ Conflicts: ^ docs/Makefile
| applications aren't ``washingtonpost.com`` or ``slashdot.org``; they're small- | ||
| to medium-sized sites with so-so traffic. But for medium- to high-traffic | ||
| applications aren't ``washingtonpost.com`` or ``slashdot.org``; they're | ||
| small-to medium-sized sites with so-so traffic. But for medium- to high-traffic |
There was a problem hiding this comment.
noticed a small typo in the commit msg: hypen -> hyphen, cheers! 👋
There was a problem hiding this comment.
Nice catch 🎯
Congrats on the role of Django Fellow! 🥳 🥳 🥳
Checking documentation in proposed patches for adherence to the 'style guide' takes time for the Fellows to review and update if required. The line length guide of 80 characters is a stand out here.
There's a tool called sphinx-lint, which err, lints sphix files. This tool's project repo seems to be fairly active and so seems well supported.
Unfortunately, it doesn't work out of the box for us as Django uses
.txtextensions and the line length check needs some adjustments to be suitable for Django's docs.This series of proposed patches does the following:
It will need some polishing, and I'm happy to do that, if there's support for this change.