-
Notifications
You must be signed in to change notification settings - Fork 187
Fix handling of dedented continuation lines. #472
Conversation
Thanks for the patch. But from what I can see, this introduces new behaviour which is unrelated to the original issue. (It looks like if the doc string now has line continuation, it no longer errors with a D207). Although this might be a good improvement, can we please file a related issue to the repo with this behaviour and reference that in this PR instead? Also it looks like most of the code in this PR is taken from #441 Can you either pick my commits from that PR to preserve the original code attribution or wait for it to be merged and create a new PR with the improved handling of line continuations? I would prefer the latter. |
Looks like the issue this PR is solving is - #234 Let's update the PR, description and docs to match the same. |
The only reason I opened this separately is because you didn't pick up #441 (comment). Now that you did I have no problem waiting for your PR to go in first and rebase this one over yours. |
The original PR was fixed. Would you like to rebase and update the description with the dedented bug fix? |
@anntzer bump? |
c91c202
to
8e947e8
Compare
fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. Sorry for the late review. Can you also add a test that tests for line continuation in sections beside the Params one?
What do you actually want to test for? |
That it doesn't throw D207 for normal doc string descriptions with line continuations |
- Dedent the function definition before parsing it. - Join continued-lines before attempting to parse the docstring.
done |
docs/release_notes.rst
Outdated
@@ -20,6 +20,8 @@ Bug Fixes | |||
The bug caused some argument names to go unreported in D417 (#448). | |||
* Fixed an issue where skipping errors on module level docstring via #noqa | |||
failed when there where more prior comments (#446). | |||
* Support backslash-continued parameter descriptions in Numpy-style docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it doing this for more than just numpy parameter descriptions at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we update this?
thanks for fixing the last part :) |
Alternative to #441; further improves handling of parameter descriptions with continued lines (which were not handled correctly before).
Thanks for submitting a PR!
Please make sure to check for the following items:
If you've added an error code or changed an error code behavior,
you should probably add or change a test case file under
tests/test_cases/
and addit to the list under
tests/test_definitions.py
.If you've added or changed a command line option,
you should probably add or change a test in
tests/test_integration.py
.Make sure to include the PR number after you open and get one.
Please don't get discouraged as it may take a while to get a review.