From 9496fc7c6a7bdfd65717eed008a347b589a519f2 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Sat, 24 Apr 2021 12:04:54 -0400 Subject: [PATCH 1/4] Fix issue #309 --- fire/docstrings.py | 6 ++++-- fire/docstrings_test.py | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 1cfadea9..dd07bb0f 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -338,7 +338,8 @@ def _as_arg_name_and_type(text): tokens = text.split() if len(tokens) < 2: return None - if _is_arg_name(tokens[0]): + if _is_arg_name(tokens[0]) and not _is_arg_name(tokens[1]): + # if _is_arg_name(tokens[0]): type_token = ' '.join(tokens[1:]) type_token = type_token.lstrip('{([').rstrip('])}') return tokens[0], type_token @@ -406,7 +407,8 @@ def _consume_google_args_line(line_info, state): state.current_arg = arg else: if state.current_arg: - state.current_arg.description.lines.append(split_line[0]) + state.current_arg.description.lines.append(first + ':' + second) + # state.current_arg.description.lines.append(split_line[0]) else: if state.current_arg: state.current_arg.description.lines.append(split_line[0]) diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index 2328ef16..1a2f4901 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -170,6 +170,33 @@ def test_google_format_multiline_arg_description(self): ) self.assertEqual(expected_docstring_info, docstring_info) + def test_google_format_multiline_arg_description_with_colon(self): + docstring = """Docstring summary. + + This is a longer description of the docstring. It spans multiple lines, as + is allowed. + + Args: + param1 (int): The first parameter. + param2 (str): The second parameter. This has a lot of text, enough to + cover two lines. This description also contains a : colon. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='Docstring summary.', + description='This is a longer description of the docstring. It spans ' + 'multiple lines, as\nis allowed.', + args=[ + ArgInfo(name='param1', type='int', + description='The first parameter.'), + ArgInfo(name='param2', type='str', + description='The second parameter. This has a lot of text, ' + 'enough to cover two lines. This description ' + 'also contains a : colon.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + def test_rst_format_typed_args_and_returns(self): docstring = """Docstring summary. From 6ad659ec63b6c916eac3694cf1644f811cbecf44 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Sat, 24 Apr 2021 15:56:13 -0400 Subject: [PATCH 2/4] Extend fix for issue #309 --- fire/docstrings.py | 11 +++++--- fire/docstrings_test.py | 56 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index dd07bb0f..4a9056d6 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -339,7 +339,6 @@ def _as_arg_name_and_type(text): if len(tokens) < 2: return None if _is_arg_name(tokens[0]) and not _is_arg_name(tokens[1]): - # if _is_arg_name(tokens[0]): type_token = ' '.join(tokens[1:]) type_token = type_token.lstrip('{([').rstrip('])}') return tokens[0], type_token @@ -393,13 +392,18 @@ def _consume_google_args_line(line_info, state): split_line = line_info.remaining.split(':', 1) if len(split_line) > 1: first, second = split_line # first is either the "arg" or "arg (type)" - if _is_arg_name(first.strip()): + new_line = line_info.indentation <= line_info.previous.indentation + if _is_arg_name(first.strip()) and ( + state.current_arg is None or new_line + ): arg = _get_or_create_arg_by_name(state, first.strip()) arg.description.lines.append(second.strip()) state.current_arg = arg else: arg_name_and_type = _as_arg_name_and_type(first) - if arg_name_and_type: + if arg_name_and_type and ( + state.current_arg is None or new_line + ): arg_name, type_str = arg_name_and_type arg = _get_or_create_arg_by_name(state, arg_name) arg.type.lines.append(type_str) @@ -408,7 +412,6 @@ def _consume_google_args_line(line_info, state): else: if state.current_arg: state.current_arg.description.lines.append(first + ':' + second) - # state.current_arg.description.lines.append(split_line[0]) else: if state.current_arg: state.current_arg.description.lines.append(split_line[0]) diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index 1a2f4901..248fc3cb 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -170,7 +170,7 @@ def test_google_format_multiline_arg_description(self): ) self.assertEqual(expected_docstring_info, docstring_info) - def test_google_format_multiline_arg_description_with_colon(self): + def test_google_format_multiline_arg_description_colon(self): docstring = """Docstring summary. This is a longer description of the docstring. It spans multiple lines, as @@ -197,6 +197,60 @@ def test_google_format_multiline_arg_description_with_colon(self): ) self.assertEqual(expected_docstring_info, docstring_info) + def test_google_format_multiline_arg_description_colon_wrapped(self): + docstring = """Docstring summary. + + This is a longer description of the docstring. It spans multiple lines, as + is allowed. + + Args: + param1 (int): The first parameter. + param2 (str): The second parameter. This description contains a + colon : after the first word of the wrapped line. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='Docstring summary.', + description='This is a longer description of the docstring. It spans ' + 'multiple lines, as\nis allowed.', + args=[ + ArgInfo(name='param1', type='int', + description='The first parameter.'), + ArgInfo(name='param2', type='str', + description='The second parameter. This description ' + 'contains a colon : after the first word ' + 'of the wrapped line.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + + def test_google_format_multiline_arg_description_colon_parenthesis(self): + docstring = """Docstring summary. + + This is a longer description of the docstring. It spans multiple lines, as + is allowed. + + Args: + param1 (int): The first parameter. + param2 (str): The second parameter. This description contains a + colon (and): parenthesis after the first word of the wrapped line. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='Docstring summary.', + description='This is a longer description of the docstring. It spans ' + 'multiple lines, as\nis allowed.', + args=[ + ArgInfo(name='param1', type='int', + description='The first parameter.'), + ArgInfo(name='param2', type='str', + description='The second parameter. This description ' + 'contains a colon (and): parenthesis after ' + 'the first word of the wrapped line.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + def test_rst_format_typed_args_and_returns(self): docstring = """Docstring summary. From 64d4a1ac5b978256deaba96a7cfa4605b6936d0e Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Sat, 24 Apr 2021 16:06:56 -0400 Subject: [PATCH 3/4] Fix style --- fire/docstrings.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 4a9056d6..7292f34e 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -392,18 +392,16 @@ def _consume_google_args_line(line_info, state): split_line = line_info.remaining.split(':', 1) if len(split_line) > 1: first, second = split_line # first is either the "arg" or "arg (type)" - new_line = line_info.indentation <= line_info.previous.indentation + new_arg = line_info.indentation <= line_info.previous.indentation if _is_arg_name(first.strip()) and ( - state.current_arg is None or new_line - ): + state.current_arg is None or new_arg): arg = _get_or_create_arg_by_name(state, first.strip()) arg.description.lines.append(second.strip()) state.current_arg = arg else: arg_name_and_type = _as_arg_name_and_type(first) if arg_name_and_type and ( - state.current_arg is None or new_line - ): + state.current_arg is None or new_arg): arg_name, type_str = arg_name_and_type arg = _get_or_create_arg_by_name(state, arg_name) arg.type.lines.append(type_str) From 434fffa812dfd90db0c619c0a36e4ef7b8ea30e4 Mon Sep 17 00:00:00 2001 From: Steven Nguyen Date: Mon, 26 Apr 2021 12:39:04 -0400 Subject: [PATCH 4/4] Make changes based on suggestions from @MichaelCG8 - Update comments in _consume_google_args_line to match new logic - Fix indentation logic: indent_check compares current line indent with first line indent for argument. - Add new function _is_arg_type to check if arg type is of valid form - Fix tests for multiline helptexts with colons and indents --- fire/docstrings.py | 29 ++++++++++++++++++++++----- fire/docstrings_test.py | 44 ++++++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/fire/docstrings.py b/fire/docstrings.py index 7292f34e..62b6bf8c 100644 --- a/fire/docstrings.py +++ b/fire/docstrings.py @@ -322,6 +322,21 @@ def _is_arg_name(name): re.match(arg_pattern, name) return re.match(arg_pattern, name) is not None +def _is_arg_type(type_): + """Returns whether type_ is a valid arg type. + + Example: + _is_arg_type("(int)") == True + + Args: + type_: The type of potential arg. + Returns: + True if type_ looks like an arg type, False otherwise. + """ + type_ = type_.strip() + return (re.match(r'^[(]\w*[)]$', type_) or + re.match(r'^[{]\w*[}]$', type_) or + re.match(r'^[\[]\w*[\]]$', type_)) def _as_arg_name_and_type(text): """Returns text as a name and type, if text looks like an arg name and type. @@ -338,7 +353,7 @@ def _as_arg_name_and_type(text): tokens = text.split() if len(tokens) < 2: return None - if _is_arg_name(tokens[0]) and not _is_arg_name(tokens[1]): + if _is_arg_name(tokens[0]) and _is_arg_type(tokens[1]): type_token = ' '.join(tokens[1:]) type_token = type_token.lstrip('{([').rstrip('])}') return tokens[0], type_token @@ -391,17 +406,21 @@ def _consume_google_args_line(line_info, state): """Consume a single line from a Google args section.""" split_line = line_info.remaining.split(':', 1) if len(split_line) > 1: - first, second = split_line # first is either the "arg" or "arg (type)" - new_arg = line_info.indentation <= line_info.previous.indentation + # first is any of the three: "arg", "arg (type)", or + # text before a colon in a continued line of an arg description + first, second = split_line + # indent_check determines if line is a new arg or + # a continuation of current arg description + indent_check = line_info.indentation <= state.section.line1_indentation if _is_arg_name(first.strip()) and ( - state.current_arg is None or new_arg): + state.current_arg is None or indent_check): arg = _get_or_create_arg_by_name(state, first.strip()) arg.description.lines.append(second.strip()) state.current_arg = arg else: arg_name_and_type = _as_arg_name_and_type(first) if arg_name_and_type and ( - state.current_arg is None or new_arg): + state.current_arg is None or indent_check): arg_name, type_str = arg_name_and_type arg = _get_or_create_arg_by_name(state, arg_name) arg.type.lines.append(type_str) diff --git a/fire/docstrings_test.py b/fire/docstrings_test.py index 248fc3cb..714fd860 100644 --- a/fire/docstrings_test.py +++ b/fire/docstrings_test.py @@ -153,7 +153,7 @@ def test_google_format_multiline_arg_description(self): Args: param1 (int): The first parameter. param2 (str): The second parameter. This has a lot of text, enough to - cover two lines. + cover two lines. """ docstring_info = docstrings.parse(docstring) expected_docstring_info = DocstringInfo( @@ -164,8 +164,8 @@ def test_google_format_multiline_arg_description(self): ArgInfo(name='param1', type='int', description='The first parameter.'), ArgInfo(name='param2', type='str', - description='The second parameter. This has a lot of text, ' - 'enough to cover two lines.'), + description='The second parameter. This has a lot of text' + ', enough to cover two lines.'), ], ) self.assertEqual(expected_docstring_info, docstring_info) @@ -179,7 +179,7 @@ def test_google_format_multiline_arg_description_colon(self): Args: param1 (int): The first parameter. param2 (str): The second parameter. This has a lot of text, enough to - cover two lines. This description also contains a : colon. + cover two lines. This description also contains a : colon. """ docstring_info = docstrings.parse(docstring) expected_docstring_info = DocstringInfo( @@ -190,9 +190,9 @@ def test_google_format_multiline_arg_description_colon(self): ArgInfo(name='param1', type='int', description='The first parameter.'), ArgInfo(name='param2', type='str', - description='The second parameter. This has a lot of text, ' - 'enough to cover two lines. This description ' - 'also contains a : colon.'), + description='The second parameter. This has a lot of text' + ', enough to cover two lines. This ' + 'description also contains a : colon.'), ], ) self.assertEqual(expected_docstring_info, docstring_info) @@ -251,6 +251,36 @@ def test_google_format_multiline_arg_description_colon_parenthesis(self): ) self.assertEqual(expected_docstring_info, docstring_info) + def test_google_format_multiline_arg_description_colon_three_lines(self): + docstring = """Docstring summary. + + This is a longer description of the docstring. It spans multiple lines, as + is allowed. + + Args: + param1 (int): The first parameter. + param2 (str): The second parameter. This has a lot of text, enough to + cover three lines. This description also contains a colon + here : on this line of the argument description. + """ + docstring_info = docstrings.parse(docstring) + expected_docstring_info = DocstringInfo( + summary='Docstring summary.', + description='This is a longer description of the docstring. It spans ' + 'multiple lines, as\nis allowed.', + args=[ + ArgInfo(name='param1', type='int', + description='The first parameter.'), + ArgInfo(name='param2', type='str', + description='The second parameter. This has a lot of text' + ', enough to cover three lines. This ' + 'description also contains a colon ' + 'here : on this line of the argument ' + 'description.'), + ], + ) + self.assertEqual(expected_docstring_info, docstring_info) + def test_rst_format_typed_args_and_returns(self): docstring = """Docstring summary.