PyCQA / docformatter

Formats docstrings to follow PEP 257
https://pypi.python.org/pypi/docformatter
MIT License
524 stars 61 forks source link

Sphinx style field lists with `.` characters in the names are formatted incorrectly #245

Closed rrwalton closed 1 year ago

rrwalton commented 1 year ago

Running the following:

> docformatter --version
docformatter 1.7.3

> cat test.py
def f(a: int) -> None:
    """Some f.
    :param a: Some param.
    :raises my.package.MyReallySrsError: Bad things happened.
    """
    return None

> docformatter test.py --diff
--- before/test.py
+++ after/test.py
@@ -1,6 +1,7 @@
 def f(a: int) -> None:
     """Some f.
-    :param a: Some param.
-    :raises my.package.MyReallySrsError: Bad things happened.
+
+    :param a: Some param. :raises my.package.MyReallySrsError: Bad
+        things happened.
     """
     return None

It seems as though the field name isn't recognised as a field name if it contains a . character. My understanding is that its valid for a sphinx style field name to contain a .; I think the SPHINX_REGEX pattern might be incorrect? https://github.com/PyCQA/docformatter/blob/b54a22caae3300f9c37f8c48666a5808d7ff8440/src/docformatter/syntax.py#L62

If I change this regex pattern to SPHINX_REGEX = r":[a-zA-Z0-9_\-(). ]*:" then I do get a correctly formatted docstring.

weibullguy commented 1 year ago

My understanding is that its valid for a sphinx style field name to contain a .; I think the SPHINX_REGEX pattern might be incorrect?

According to the reStructuredText Markup Specification for Field Lists:

A field name may consist of any characters, but colons (":") inside of field names must be backslash-escaped when followed by whitespace.

And per the Sphinx documentation for Field Lists

Sphinx extends standard docutils behavior for field lists and adds some extra functionality that is covered in this section.

Thus, your understanding is correct and the SPHINX_REGEX pattern needs to account for . in the field name.

rrwalton commented 1 year ago

If the field name could be any characters, with a special case of 'colons followed by white-space' being backslash-escaped, then I think the regexp probably needs some more work than in my suggested fix. I'm not really a regex whizz, but there's possibly an incantation that'll capture all the cases?

It might also need to handle strings like: :raises `my.package.Error`: Raised when :py:obj:`my.package.RobustObject` sees something it doesn't like.

Although I'm not sure how much the sphinx cross-referencing stuff is in scope for this tool, because it's not really anything to do with pep257.