Do not double-process urls (#284)

* add failing test

* assume that all urls after first field are in field list

* fix modifying iterated collection causing skips

* add test for epytext style too

* add some unit tests for various URL scenarios

Author: lilatomic

src/docformatter/syntax.py

@@ -950,7 +950,7 @@ def _field_over_url(
     field_idx: List[Tuple[int, int]],
     url_idx: List[Tuple[int, int]],
 ):
-    """Remove URL indices that overlap with filed list indices.
+    """Remove URL indices that overlap with field list indices.
 
     Parameters
     ----------
@@ -965,14 +965,14 @@ def _field_over_url(
         The url_idx list with any tuples that have indices overlapping with field
         list indices removed.
     """
-    for _fieldl, _fieldu in field_idx:
-        for _key, _value in enumerate(url_idx):
-            if (
-                _value[0] == _fieldl
-                or _value[0] == _fieldu
-                or _value[1] == _fieldl
-                or _value[1] == _fieldu
-            ):
-                url_idx.pop(_key)
-
-    return url_idx
+    if not field_idx:
+        return url_idx
+
+    nonoverlapping_urls = []
+
+    any_param_start = min(e[0] for e in field_idx)
+    for _key, _value in enumerate(url_idx):
+        if _value[1] < any_param_start:
+            nonoverlapping_urls.append(_value)
+    return nonoverlapping_urls
+

tests/_data/string_files/do_format_docstrings.toml

@@ -213,3 +213,29 @@ instring='''"""
 eBay kinda suss
 """'''
 outstring='''"""eBay kinda suss."""'''
+
+[issue_263]
+[issue_263.sphinx]
+# the `xx.\n\n` ensures there are a summary and a description sections
+# the `:param a:` creates a field
+# the `b`s create text that is long enough to trigger a line wrap without being so long that they count as code
+# the `s3://cccc.` is a url
+instring='''"""xx.
+
+    :param a: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb s3://cccc.
+"""'''
+outstring='''"""xx.
+
+    :param a: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+        s3://cccc.
+    """'''
+[issue_263.epytext]
+instring='''"""xx.
+
+    @param a: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb s3://cccc.
+    """'''
+outstring='''"""xx.
+
+    @param a: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+        s3://cccc.
+    """'''

tests/formatter/test_do_format_docstring.py

@@ -337,3 +337,24 @@ def test_format_docstring_with_non_cap_words(self, test_args, args):
             INDENTATION,
             instring,
         )
+
+    @pytest.mark.unit
+    @pytest.mark.parametrize("args", [["--style", "sphinx", ""], ["--style", "epytext", ""]])
+    def test_do_not_double_process_urls(self, test_args, args):
+        """Do not double-process urls in fields
+
+        See issue #263
+        """
+        style = args[1]
+
+        uut = Formatter(
+            test_args,
+            sys.stderr,
+            sys.stdin,
+            sys.stdout,
+        )
+
+        instring = self.TEST_STRINGS["issue_263"][style]["instring"]
+        outstring = self.TEST_STRINGS["issue_263"][style]["outstring"]
+
+        assert outstring == uut._do_format_docstring(INDENTATION, instring, )

tests/test_syntax_functions.py

@@ -34,12 +34,14 @@
     do_find_links()
     do_skip_link()
 """
+import textwrap
 
 # Third Party Imports
 import pytest
 
 # docformatter Package Imports
 import docformatter
+from docformatter import do_split_description
 
 
 class TestURLHandlers:
@@ -159,3 +161,76 @@ def test_find_double_backtick_directives(self):
             "    Since a mail could be ``Cc``'d to two lists with different ``Reply-To`` munging"
             "options set."
         )
+
+
+class TestSplitDescription:
+    """Class for testing the function to process the description
+
+    Includes tests for:
+        - do_split_description()
+
+    """
+    med_str = "m"*40  # long enough that 2 won't fit on a line
+    indent = "    "
+
+    def do_test(self, text):
+        return do_split_description(textwrap.dedent(text), self.indent, 72, "sphinx")
+
+    def indent_all(self, strs):
+        return [self.indent + s for s in strs]
+
+    @pytest.mark.unit
+    def test_split_description_url_outside_param(self):
+        assert self.do_test(
+            f"""\
+            {self.med_str} https://{self.med_str}
+            :param a: {self.med_str}
+            """
+        ) == self.indent_all([
+            self.med_str,
+            f"https://{self.med_str}",
+            f":param a: {self.med_str}",
+        ])
+
+    @pytest.mark.unit
+    def test_split_description_single_url_in_param(self):
+        assert self.do_test(
+            f"""\
+            {self.med_str}
+            :param a: {self.med_str} https://{self.med_str}a
+            """
+        ) == self.indent_all([
+            self.med_str,
+            f":param a: {self.med_str}",
+            self.indent + f"https://{self.med_str}a",
+        ])
+
+    @pytest.mark.unit
+    def test_split_description_single_url_in_multiple_params(self):
+        assert self.do_test(
+            f"""\
+            {self.med_str}
+            :param a: {self.med_str} https://{self.med_str}a
+            :param b: {self.med_str} https://{self.med_str}b
+            """
+        ) == self.indent_all([
+            self.med_str,
+            f":param a: {self.med_str}",
+            self.indent + f"https://{self.med_str}a",
+            f":param b: {self.med_str}",
+            self.indent + f"https://{self.med_str}b",
+        ])
+
+    @pytest.mark.unit
+    def test_split_description_multiple_urls_in_param(self):
+        assert self.do_test(
+            f"""\
+            {self.med_str}
+            :param a: {self.med_str} https://{self.med_str}0 https://{self.med_str}1
+            """
+        ) == self.indent_all([
+            self.med_str,
+            f":param a: {self.med_str}",
+            self.indent + f"https://{self.med_str}0",
+            self.indent + f"https://{self.med_str}1",
+        ])