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

bpo-42238: Check Misc/NEWS.d/next/ for reStructuredText issues. #23802

Merged
merged 3 commits into from
Dec 18, 2020

Conversation

JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented Dec 16, 2020

This is to avoid things like: #23800

https://bugs.python.org/issue42238

Doc/tools/rstlint.py Outdated Show resolved Hide resolved
line = "\n"
else:
in_multiline_comment = False
if line.startswith(".. "):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I'm misinterpreting something here, but wouldn't this remove directives like .. describes:: or .. class:: too? Or is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're understanding it correctly, it removes a bit too much. But the documentation tells comments are Every explicit markup block which isn’t a valid markup construct.

an explicit markup is:

An explicit markup block begins with a line starting with .. followed by whitespace and is terminated by the next paragraph at the same level of indentation.

According to the ref, explicite markup are

used for footnotes, citations, hyperlink targets, directives, substitution definitions, and comments

in directives there's potentially a lot, as it's possible to add new directives, like versionchanged in our doc.

So we have like three choices:

  • Keep it simple and introduce some false negatives (current version)
  • Do it right with no false positives and no false negative, it's probably doable as we have the exhaustive list of directives already hardcoded in rstlint.py
  • Keep it simple, the other way around, and introduce some false positives.

I'm really not in favor of false positives: it's a burden for contributors. False negatives are OK though: if an rst error is introduced it'll still fixable later.

I'll try myself at the "do it right version" today to see if it's worth it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not that hard, probably worth it, I pushed it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience and explaining it to me :).

JulienPalard and others added 2 commits December 17, 2020 10:55
Co-authored-by: kj <28750310+Fidget-Spinner@users.noreply.github.com>
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Once again, thanks for making this :)

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.

4 participants