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

Add check for missing space after directive :: #8

Closed
wants to merge 1 commit into from
Closed

Add check for missing space after directive :: #8

wants to merge 1 commit into from

Conversation

jeanas
Copy link
Contributor

@jeanas jeanas commented Feb 23, 2022

For example:

.. versionadded::0.1

This is taken as a comment, thus no warning from docutils/Sphinx.

This doesn't yield any false positives in CPython.

Closes #7


This is a first PR to get familiar with the script.

For example:

.. versionadded::0.1

This is taken as a comment, thus no warning from docutils/Sphinx.

This doesn't yield any false positives in CPython.

Closes #7
@JulienPalard
Copy link
Collaborator

Found a false positive in kitchen-sink:

Centered text
-------------

You can create a statement with centered text with ``.. centered::``

.. centered:: This is centered text!

It may be solved by adding ^ * in front of the regex maybe? It's not perfect as it won't handle tables, like:

+=============================+=============================+======================+
| :c:type:`allocfunc`         | .. line-block::             | :c:type:`PyObject` * |
|                             |                             |                      |
|                             |    :c:type:`PyTypeObject` * |                      |
|                             |    Py_ssize_t               |                      |
+-----------------------------+-----------------------------+----------------------+

but we know we don't handle tables yet.

@jeanas
Copy link
Contributor Author

jeanas commented Mar 7, 2022

Darn, good catch. Actually, running sphinx-lint in the documentation of Sphinx itself, I get tons of false positives ("Missing backslash-space between literal and text") for similar stuff like

``:math:`α```

Maybe hide_non_rst_blocks should start removing literal inline markup?

@JulienPalard
Copy link
Collaborator

JulienPalard commented May 23, 2022

Heads up: sphinx-lint should soon pass consistently on sphinx-doc: sphinx-doc/sphinx#10389

@@ -119,6 +119,10 @@
# .. versionchanged:: 3.6
three_dot_directive_re = re.compile(r"\.\.\. %s::" % all_directives)

# Find directive missing space after double colon, like:
# .. versionchanged::3.6
missing_space_directive_re = re.compile(r"(?<!\.)\.\. %s::[^\s]" % simplename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
missing_space_directive_re = re.compile(r"(?<!\.)\.\. %s::[^\s]" % simplename)
missing_space_directive_re = re.compile(r"(?<!\.)\.\. %s::[^`\s]" % simplename)

Would this tweak be enough?

>>> simplename = r"(?:(?!_)\w)+(?:[-._+:](?:(?!_)\w)+)*"
>>> missing_space_directive_re = re.compile(r"(?<!\.)\.\. %s::[^`\s]" % simplename)
>>> missing_space_directive_re.match('`.. foo::123`')
>>> missing_space_directive_re.match('``.. foo::123``')
>>> missing_space_directive_re.match('``.. foo::``')
>>> missing_space_directive_re.match(':dir:`.. foo::`')
>>> missing_space_directive_re.match('.. foo::')
>>> missing_space_directive_re.match('.. foo:: ')
>>> missing_space_directive_re.match('| .. foo:: |')
>>> missing_space_directive_re.match('.. foo::`bar`')
>>> missing_space_directive_re.match('.. foo:: valid')
>>> missing_space_directive_re.match('.. foo::invalid')
<re.Match object; span=(0, 9), match='.. foo::i'>
>>> missing_space_directive_re.match('.. foo::3.7 invalid')
<re.Match object; span=(0, 9), match='.. foo::3'>

It catches the last two (invalid), and ignores all the other ones. It will miss something like .. foo::`bar` (the user might have wanted .. foo:: `bar`, which should be valid), but this seems quite unlikely.

@jeanas jeanas closed this by deleting the head repository Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for missing space before directive arguments
3 participants