Skip to content

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Aug 1, 2023

Introduce docstring_append() helper, and use it for both parameter and
function docstrings. Remove docstring fixup from
do_post_block_processing_cleanup(); instead, make sure no fixup is needed.

…g' states Introduce docstring_append() helper, and use it for both parameter and function docstrings. Remove docstring fixup from do_post_block_processing_cleanup(); instead, make sure no fixup is needed.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove docstring fixup from
do_post_block_processing_cleanup(); instead, make sure no fixup is needed.

I think the fixup is still needed. If I apply this diff to your PR branch:

--- a/Tools/clinic/clinic.py+++ b/Tools/clinic/clinic.py@@ -5577,6 +5577,11 @@ def do_post_block_processing_cleanup(self) -> None: if no_parameter_after_star: fail("Function " + self.function.name + " specifies '*' without any parameters afterwards.") + for name, value in self.function.parameters.items():+ if not value:+ continue+ assert value.docstring == value.docstring.rstrip()+ self.function.docstring = self.format_docstring()

Then the assertion fails in ClinicParserTest.test_function_docstring:

====================================================================== FAIL: test_function_docstring (test.test_clinic.ClinicParserTest.test_function_docstring) ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\Users\alexw\coding\cpython\Lib\test\test_clinic.py", line 632, in test_function_docstring function =self.parse_function("""^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\alexw\coding\cpython\Lib\test\test_clinic.py", line 1384, in parse_function block =self.parse(text) ^^^^^^^^^^^^^^^^ File "C:\Users\alexw\coding\cpython\Lib\test\test_clinic.py", line 1380, in parse parser.parse(block) File "C:\Users\alexw\coding\cpython\Tools\clinic\clinic.py", line 4612, in parseself.do_post_block_processing_cleanup() File "C:\Users\alexw\coding\cpython\Tools\clinic\clinic.py", line 5583, in do_post_block_processing_cleanupassert value.docstring == value.docstring.rstrip() AssertionError ----------------------------------------------------------------------

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@erlend-aasland
Copy link
ContributorAuthor

I think the fixup is still needed. [...]

The docstrings are further processed in format_docstring; I'm not sure it matters that they are off at this point in the parsing.

@AlexWaygood
Copy link
Member

I think the fixup is still needed. [...]

The docstrings are further processed in format_docstring; I'm not sure it matters that they are off at this point in the parsing.

Ugh, yes, I think you're right :)

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks good now the test's been added :)

@erlend-aasland
Copy link
ContributorAuthor

Ugh, yes, I think you're right :)

See also the added test ;)

@erlend-aaslanderlend-aasland enabled auto-merge (squash) August 1, 2023 22:58
@erlend-aaslanderlend-aasland merged commit b4d8897 into python:mainAug 1, 2023
@erlend-aaslanderlend-aasland deleted the clinic/docstring-append branch August 22, 2023 08:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@erlend-aasland@bedevere-bot@AlexWaygood