Add sphinx-lint to the ci workflow and Makefile.

[ezio-melotti]

This is an attempt to add a linter, as suggested in #494.

[cschan1828]

I suggest that we run the lint tool before proceeding with the full HTML build. This way, we can detect any issues earlier.

Also, I am concerned about potential false alarms that may block pull requests. @mattwang44 currently do we have a such setting that requires all checks to pass before PR merge?

[mattwang44]

currently do we have a such setting that requires all checks to pass before PR merge?

yes, and I tend not to change the setting.

I think we should look into the current lint issues first before merging this PR. The one in the readme.rst can be ignored (just fix it) but the other two in sphinx.po seem to be false alarms. Not sure if sphinx-lint has support any syntax that allows users to ignore the issues (like # noqa for most linters)

[ezio-melotti]

I suggest that we run the lint tool before proceeding with the full HTML build. This way, we can detect any issues earlier.

I now moved this before the validate step in 6f7d53b. I also verified that sphinx-lint is able to detect issues directly in .po files, and even if it needs to convert them to .rst first, the whole process (converting + checking) takes around 10-15s, which is IMHO acceptable.

Also, I am concerned about potential false alarms that may block pull requests.

sphinx-lint only detected 3 errors, and now that I added an additional check (in f257955) it detected 4 additional errors, bringing the total up to 7. This is a pretty low number, which is probably explained by the fact that we also run sphinx-lint on the English CPython docs.

I'm looking at the sphinx.po issues to determine what the problem is. If it turns out there are indeed false positives, we should report and fix them upstream. I can also mark the "Lint" step as optional, so that it doesn't prevent merges if there are failures, but once the false positives are fixed I'd rather keep it as a mandatory step.

I can also prepare another PR to fix all the issues so that we can merge it before this PR.

Finally, in c5462a9, I added a push trigger to CI so that the the workflow is run again as I push more changes. I think it would be good to have it anyway, but since it doesn't actually belong to this PR, I can remove it and possibly add it as a separate PR if you agree on having it.

[cschan1828]

Could we just add Makefile to this commit? If so, I agree to merge this PR.

I don't think we are ready to integrate this with CI. The benefits are not clear now, we still need to check many common errors specific to CJK text.

[ezio-melotti]

Could we just add Makefile to this commit? If so, I agree to merge this PR.

My plan is to leave the CI for now so that we can see if the changes we make work or not -- I can remove it before merging.

I now backported python/cpython#107067:

• [3.12] Remove superflous whitespaces in layout.html. (GH-107067) cpython#107251
• [3.11] Remove superflous whitespaces in layout.html. (GH-107251) cpython#107252

Next I'll fix the other errors.

[mattwang44]

I've fixed the trailing whitespace issue in sphinx.po (#504)

[cschan1828]

@ezio-melotti may you remove this? We can provide this linter in our tool set but I don't think it is ready for production use.

[ezio-melotti]

I now removed this. Let me know if there is anything else I should do before we merge this. You can also ping me on Discord if you want to talk about what should be changed in the workflow and/or sphinx-lint to make it suitable for this repo.

[mattwang44]

It looks great! Thanks, Ezio!
Maybe the next step is trying to use it as a pre-commit hook (ref).

[mattwang44]

@cschan1828 I believe we can merge this PR, right?

[cschan1828]

Sorry for late response. It looks good to me. Thanks for your contribution!