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

Bugs in link handling in 1.5.1 #140

Closed
drasmuss opened this issue Dec 20, 2022 · 6 comments · Fixed by #141 or kdeldycke/workflows#397
Closed

Bugs in link handling in 1.5.1 #140

drasmuss opened this issue Dec 20, 2022 · 6 comments · Fixed by #141 or kdeldycke/workflows#397
Labels
C: style Relates to docstring format style (e.g., Google, NumPy, Sphinx) P: bug PEP 257 violation or existing functionality that doesn't work as documented S: merged Closed with work merged to repository
Milestone

Comments

@drasmuss
Copy link

drasmuss commented Dec 20, 2022

I've run into a couple issues related to formatting links in 1.5.1.

  1. Error with short link

To reproduce, create a file tmp.py containing:

"""
Test test.

See `the link <https://www.link.com>`_ for more details.
"""

Running docformatter tmp.py gives an error

   ...
  File ".../lib/python3.9/site-packages/docformatter/syntax.py", line 272, in wrap_description
    text = do_preserve_links(text, indentation, wrap_length)
  File ".../lib/python3.9/site-packages/docformatter/syntax.py", line 82, in do_preserve_links
    url = url + lines[url_idx + 1].strip()
IndexError: list index out of range
  1. Long links that get broken across lines also remove spaces from the text following the link
"""
Test test.

See `the link <https://www.link.com/a/long/link/that/causes/line/break>`_ for more details.
"""

Running docformatter tmp.py produces the output

--- before/tmp.py
+++ after/tmp.py
@@ -1,5 +1,6 @@
+"""Test test.
+
+See `the link
+
+<https://www.link.com/a/long/link/that/causes/line/break>`_ for moredetails.
 """
-Test test.
-
-See `the link <https://www.link.com/a/long/link/that/causes/line/break>`_ for more details.
-"""

Note that the space has been removed between "more" and "details".

@github-actions github-actions bot added the fresh This is a new issue label Dec 20, 2022
@weibullguy weibullguy added P: bug PEP 257 violation or existing functionality that doesn't work as documented C: style Relates to docstring format style (e.g., Google, NumPy, Sphinx) V: patch Bumps the patch version and removed fresh This is a new issue labels Dec 21, 2022
@weibullguy weibullguy added this to the v1.6.0 milestone Dec 21, 2022
@kdeldycke
Copy link

Can confirm the issue, both with docformatter 1.5.0 and 1.5.1.

Trace of my CI for reference: https://github.com/kdeldycke/click-extra/actions/runs/3786046355/jobs/6436595023#step:8:43

@weibullguy
Copy link
Member

weibullguy commented Dec 29, 2022

For the first problem (IndexError), docformatter shouldn't even be preserving links unless the wrapping broke it across multiple lines. PR #141 now causes docformatter to skip link preservation in any wrapped text that results in no wrapped lines.

For the second problem, rebuilding the link should've resulted in the following:

../test.py
--- before/../test.py
+++ after/../test.py
@@ -8,6 +8,7 @@
 def b_function():
     """Another test docstring.
 
-    See `the link <https://www.link.com/a/long/link/that/causes/line/break>`_ for more details.
+    See `the link
+    <https://www.link.com/a/long/link/that/causes/line/break>`_ for more
+    details.
     """

Note that it added a blank line after the See `the link line and should have placed details on a line of it's own because the line ending with more was at the wrap length. PR #141 now causes docformatter to produce the results above.

Thanks @drasmuss and @kdeldycke for your inputs and sorry for any headaches.

@weibullguy weibullguy added the S: merged Closed with work merged to repository label Dec 29, 2022
@kdeldycke
Copy link

Thanks @weibullguy for being so fast fixing this issue! I'm relying on docformatter < 1.5 while I wait for the next release, so no worries.

@AlexTereshenkov
Copy link

@weibullguy in case this is of any help, 1.5.1 trips over a particular URL:

class Klass():
    """Helpful docstring.

    A larger description that starts here.
    https://github.com/some/rather/long/url/file.ext
    A larger description that ends here.
    """
    pass

is reformatted fine.

However

class Klass():
    """Helpful docstring.

    A larger description that starts here.
    https://github.com/apache/kafka/blob/2.5/clients/src/main/java/org/apache/kafka/common/requests/DescribeConfigsResponse.java
    A larger description that ends here.
    """
    pass

is losing data during the reformat operation, the output:

class Klass():
    """Helpful docstring.

    A
    largermmon/requests/DescribeConfigsResponse.java
    A larger description that ends here.
    """
    pass

Not sure if this case is handled by #141 as the URL is not in RST style, so thought worth posting for you (and perhaps others).

@AlexTereshenkov
Copy link

AlexTereshenkov commented Jan 10, 2023

A different, but related case with the URL:

def func():
    """Do something.

    See https://www.postgresql.org/docs/current/static/role-removal.html
    """
    return

1.5.1 trips over with the traceback (with no configuration at all, i.e. all defaults):

...
  File "venv/lib64/python3.10/site-packages/docformatter/syntax.py", line 98, in do_preserve_links
    url = url + lines[url_idx + 1].strip().split(sep=" ")[0].strip()
IndexError: list index out of range

@weibullguy weibullguy removed the V: patch Bumps the patch version label Jan 19, 2023
@weibullguy
Copy link
Member

@drasmuss, @kdeldycke, and @AlexTereshenkov I got ahead of myself releasing 1.5.0 to PyPi without sufficient testing. Before I cause users anymore headaches, I'm providing release candidates for the brave to use and provide feedback. If you'd like to use the next release candidate that should resolve your issue, you can install it with pip:

pip install git+https://github.com/PyCQA/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: style Relates to docstring format style (e.g., Google, NumPy, Sphinx) P: bug PEP 257 violation or existing functionality that doesn't work as documented S: merged Closed with work merged to repository
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants