From a850cf98e8016ae9a36ce5c4ad882e87e9e74b45 Mon Sep 17 00:00:00 2001 From: Daniel Goldman Date: Wed, 31 Jul 2024 10:08:24 -0400 Subject: [PATCH] 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 --- src/docformatter/syntax.py | 24 +++--- .../string_files/do_format_docstrings.toml | 26 +++++++ tests/formatter/test_do_format_docstring.py | 21 ++++++ tests/test_syntax_functions.py | 75 +++++++++++++++++++ 4 files changed, 134 insertions(+), 12 deletions(-) diff --git a/src/docformatter/syntax.py b/src/docformatter/syntax.py index a389341..f375172 100644 --- a/src/docformatter/syntax.py +++ b/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 + diff --git a/tests/_data/string_files/do_format_docstrings.toml b/tests/_data/string_files/do_format_docstrings.toml index d8d29d9..5cc1735 100644 --- a/tests/_data/string_files/do_format_docstrings.toml +++ b/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. + """''' diff --git a/tests/formatter/test_do_format_docstring.py b/tests/formatter/test_do_format_docstring.py index 18b65d7..5bf544b 100644 --- a/tests/formatter/test_do_format_docstring.py +++ b/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, ) diff --git a/tests/test_syntax_functions.py b/tests/test_syntax_functions.py index 235bb45..24d45a9 100644 --- a/tests/test_syntax_functions.py +++ b/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", + ])