Docstring description multiline parsing

[thebadcoder96]

this would potentially close #448

Please review.

[thebadcoder96]

Sorry I was slacking off a bit :P

I have found another issue with the code (a new possible test case). When I use : in the second or third line, the line disappears since it is detected as an arg with the current logic. I also found that my line3 logic to check if it was an arg was wrong. Below is the example for the : issue.

Ex: if I had something like this:

    param3_that_is_very_longer_than_usual: The second parameter. This has a lot of text,
      enough to: cover two lines.

it would print out:

PARAM3_THAT_IS_VERY_LONGER_THAN_USUAL
        The second parameter. This has a lot of text, 

The problem is with _as_arg_name_and_type() because when the part split by : passes through it, it only checks if there is more than 1 string split by ' ' and if the first word looks like an arg, combines everything else into a string removing ()[]{} (out current logic). I have added another condition to check if these brackets are present to qualify the whole string as an arg and type in that function.

When I made this change I noticed that around line 408/417 within _consume_google_args_line()

      else:
        if state.current_arg:
          state.current_arg.description.lines.append(split_line[0])

Here we are appending only split_line[0] but we should be joining everything back with : and then appending it. I made this change as well. Then I realized that we never do the check for line2 and line3 and store their values (arg states) here so if there is a line with : that is not an arg it does not check for the code I have added.

I decided to convert the check into a function and call them in both places when the line is not an arg. Added 3 more tests regarding the :

[dbieber]

Ack. That all sounds good. Will update you when I get to the review.

On Sat, Jan 13, 2024 at 9:57 PM Mishahal Palakuniyil < @.***> wrote:

@.**** commented on this pull request.

In fire/docstrings.py https://github.com/google/python-fire/pull/476#discussion_r1451644568:

@@ -410,6 +419,83 @@ def _consume_google_args_line(line_info, state): else: if state.current_arg: state.current_arg.description.lines.append(split_line[0])

• state.max_line_length = max(state.max_line_length, len(line_info.line))
• if line_info.previous.line == state.line1: # check for line2

I have added the index as you guided and updated the code to compare indexes instead of strings. Please review the most recent commit.

— Reply to this email directly, view it on GitHub https://github.com/google/python-fire/pull/476#discussion_r1451644568, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGZ6XKTW2ZEX7I5P7DXH4TYONCTHAVCNFSM6AAAAABBAQ6XUKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMRQGEZTKOBUGM . You are receiving this because you commented.Message ID: @.***>

[thebadcoder96]

Ack. That all sounds good. Will update you when I get to the review.

What I mentioned might be another bug. I also just thought about this; I was messing around adding : in the second and third lines of the description. I always added it after 2 words (like in the example mentioned above) which caused me to believe _as_arg_name_and_type was the problem.

I haven't tested this, but if we add : after the first word in descriptions (line2 or later), the _is_arg_name() function will be called and according to our current logic, anything that is just a word will be considered as an arg. I think we want a better way to identify an arg.

On the top of my head, I'm thinking we can do something like compare the previous line if state.current_arg exists and see if there is a difference in indentation (most likely not an arg if line2 first word and arg name indentation difference is present). Just spit-balling my thoughts right now.