Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compat removal and splitlines cleanup v2 #111

Merged
merged 9 commits into from
Jan 7, 2023

Conversation

jnikula
Copy link
Owner

@jnikula jnikula commented Jan 7, 2023

v2 of #110

The compat features have been deprecated since 0.8.0. Time to stop using
them in tests.
It's been deprecated since 0.8.0.
Start localizing line splitting in one place in docstring. There's
already some splitting and joining going on in _get_plain_comment(), so
this helps us reduce them in the future.

Add a blank line in the end if there isn't one. This unifies the number
of blank lines in the napoleon transform test case, reducing an extra
blank line.

As the whitespace (tabs to spaces) conversion is moved earlier, we also
need to adjust a few tests to account for this. We'll further adjust
this in follow-up.
Doing whitespace conversion (tabs to spaces) after the content has been
indented gets tab stops wrong. What looks like good indentation in the C
source might not be in rst. Move whitespace conversion earlier.
@@ -130,19 +130,22 @@ def _nest(text, nest):
Returns:
str: Indented reST documentation string.
"""
return re.sub('(?m)^(?!$)', ' ' * nest, text)
lines[:] = [re.sub('^(?!$)', ' ' * nest, line) for line in lines]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I missed it before, but the docstring of the function mentions a return value.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Force pushed with the fix.

For a code documentation tool we have pretty lousy code documentation ourselves. :D

@jnikula
Copy link
Owner Author

jnikula commented Jan 7, 2023

Btw I also considered removing the newlines from all the _fmt strings, and making them just the directive. Then _get_header_lines() would add the newlines. But I left that up to you and/or for the future.

Reduce the number of line splitting and joining, and start operating on
lists of lines. This means formatting the header separately from the
rest of the comment.
The regex was handy for multiline replacements for nesting, but as we
now operate on a list of lines, we can just do this manually in a
cleaner way.
Move even more towards building blocks that operate on lists of lines.
Add a separate function for formatting the directive, or header, lines.
Make the transform function passed to get_docstring() handle a list of
strings instead of a multiline string. For now, this is a bit clumsy
outside of docstring, but we'll clean it up later.
@jnikula jnikula force-pushed the compat-removal-and-splitlines-cleanup-v2 branch from d2da006 to 5dd106f Compare January 7, 2023 14:49
@BrunoMSantos
Copy link
Collaborator

Btw I also considered removing the newlines from all the _fmt strings, and making them just the directive. Then _get_header_lines() would add the newlines. But I left that up to you and/or for the future.

That may be cleaner overall actually. It gets rid of the splitlines() for one thing.

It's nitpicking though, so I'll leave it up to you. This is a good set of changes regardless. +1

@jnikula jnikula merged commit 2264238 into master Jan 7, 2023
@jnikula jnikula deleted the compat-removal-and-splitlines-cleanup-v2 branch January 7, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants