close
The Wayback Machine - https://web.archive.org/web/20210202062753/https://github.com/python/peps/pull/1668
Skip to content
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

Lint RST inline code on GitHub Actions #1668

Open
wants to merge 15 commits into
base: master
from

Conversation

@hugovk
Copy link
Contributor

@hugovk hugovk commented Oct 21, 2020

(Split from #1570.)

This PR adds two reStructuredText lint rules and runs them on the CI using GitHub Actions. Example run.

  1. Detect single backticks, which should be double in RST for inline code, e.g. stuff fixed in #1554.

  2. Detect inline code touching normal text, e.g. stuff fixed in #1560.


This uses the pre-commit tool for linting. It's possible, but completely optional, to use it locally and it'll check your changes when you're committing, and fail before commit if something is found.

Optional local set up (run once):

pip install -U pre-commit && pre-commit install

Then make changes and commit as normal:

git commit -m "PEP XXX: etc"

Lint all files:

pre-commit run --all-files

# Or this does the same thing
make lint

The very first run takes a little while to set things up, after that it's quick.

But again, local use of pre-commit is completely optional for those who aren't keen, and the CI will run the checks anyway.


The first lint rule finds some false positives:

pep-0012.rst:602:    `single-quoted' or ``double-quoted''
pep-0505.rst:245:    Total None-coalescing `if` blocks: 449
pep-0505.rst:246:    Total [possible] None-coalescing `or`: 120
pep-0505.rst:248:    Total Safe navigation `and`: 13
pep-0505.rst:249:    Total Safe navigation `if` blocks: 61
pep-0550.rst:540:    g = gen()  # gen_LC is created for the generator object `g`
pep-0550.rst:763:        # This class is what the `gen_series()` generator can
pep-0550.rst:772:            # Otherwise `var.set(10)` in the `_init` method
pep-0550.rst:857:        # in the `Context Variables` section.
  • pep-0012.rst is an example of what not to do! So that needs to stay.

  • pep-0505.rst is example output from a command, so (most likely) needs to stay.

There's no way to exclude single lines from the linter, so both these files have been excluded.

  • Backticks already in code: I've changed them to single quotes. Because it's already formatted with code, no extra formatting was added in the rendered monospace output, so single quotes can also be used for the emphasis.

  • After rebasing on master, this found a few other single backticks which should have been double backticks or links. I've fixed them.

@hugovk hugovk force-pushed the hugovk:lint-rsk-backticks-only branch from d9207ad to 8b8057c Feb 1, 2021
@hugovk
Copy link
Contributor Author

@hugovk hugovk commented Feb 1, 2021

Rebased and fixed new broken backticks in 6 more PEPs.

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Feb 1, 2021

So I'm having a hard time understanding why we need to touch code blocks at all. AFAICT backticks there are valid (they certainly work in practice). Is this just a deficiency in the linter?

I'd be happy to accept changes that fix actually incorrect uses of backticks outside of code blocks (you found several of those), but for the code blocks I think the linter should be fixed first, or you have to convince me that backticks in code blocks are invalid ReST.

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

Successfully merging this pull request may close these issues.

None yet

3 participants