Add option to format compatible with black:

NickHilton commented 1 year ago

Summary

Attempts to address the issue in https://github.com/PyCQA/docformatter/issues/94 by adding a CLI arg to allow compatibility with black. This does not implement everything (i.e. pyproject.toml specifications) as I wanted feedback before continuing with this to finish the PR.

This adds a --black argument to the CLI args which, when passed, ensure that the issue from #94 is addressed. I'm not aware of any further discrepancies with black which need to be addressed but please let me know if there are

Updated I also added a --black-line-length arg, only available if you specify --black which will automatically set --wrap-summaries and --wrap-descriptions to the value specified whilst defaulting to 88, consistent with black. I wasn't sure this was strictly necessary as the changes would be compatible with black even setting custom line lengths, however this was raised in the original issue as desired

Per comments - have left default behaviour of line length as 88 when --black is specified, otherwise use the current PEP8 defaults, and allow the user to specify wrap-summaries and wrap-descriptions as desired to overwrite this.

I added a unittest and tested the full script passing in the --black argument and got an expected result

Changes

Adds CLI arg --black to respect black formatting rules
Adds CLI arg --black-line-length to format line lengths consistently and with the black default
Updates _do_format_docstring to respect black formatting rules w.r.t. leading "
Adds unittest for _do_format_docstring for the black case
Updates docs

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4326573190

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at ?%

Totals
Change from base Build 4326535638:	0%
Covered Lines:
Relevant Lines:	0

💛 - Coveralls

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4326573190

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 0.0%

Totals
Change from base Build 4326535638:	0.0%
Covered Lines:	0
Relevant Lines:	0

💛 - Coveralls

s-weigand commented 1 year ago

Another necessary change for docformatter to play nicely with black is that _do_remove_blank_lines_after_method does not get executed (see #161 )

As for configuration, it would be nice if a config like this would work. Or line-length would get extracted from the black config section.

[tool.black]
line-length = 99

[tool.docformatter]
black = true
wrap-summaries = 99
wrap-descriptions = 99

Sorry for the unasked 2 cents, but docformatter and black fighting about formatting when running pre-commit is the reason why I'm sticking with docformatter==1.5.0 right now 😅

weibullguy commented 1 year ago

@s-weigand I think you'll see from my comments, that's exactly the way I'd like docformatter to work. As far as using options for another tool to modify docformatter's behavior, I'm not 100% sold on that. But, I'm not 100% against it either. As I stated in my review comments above,

The only thing --black NEEDS to modify is the --pre-summary-space behavior because docformatter already has arguments for line length.

Adding two lines, one time to a configuration file doesn't seem overly burdensome to me (of course, I walked up hill both ways to school when I was a kid, so....). This keeps the tools configurations separate which is what I prefer until convinced otherwise.

weibullguy commented 1 year ago

@NickHilton @s-weigand pointed out a behavior that needs to be modified by the --black argument. In format.py the Formatter._format_code method has the line:

modified_tokens = self._do_remove_blank_lines_after_method(
    modified_tokens
)

this should not be executed when the --black argument is passed. See #161.

NickHilton commented 1 year ago

I have addressed the blank lines after method issue:

When black runs it strips blank lines after method docstrings up to 1 blank line.

I wanted to replicate that behaviour rather than not format the blank lines at all after method docstrings:

To be consistent with black
To be consistent with the other behaviour of docformatter which includes formatting blank lines around the docstring as inside the scope of what docformatter will change

NickHilton commented 1 year ago

I presume the tests are passing on your end? For some reason they're not on GH. Also, there are some pycodestyle errors that need addressing.

The one thats failing does... Although I can't get all the tests to pass on master branch so probably have something odd with my local set up. Will see if I can get it fixed

Will address the pycodestyle errors

weibullguy commented 1 year ago

@NickHilton I just released v1.6.0. You'll want to merge those changes and make sure everything is still working.

NickHilton commented 1 year ago

Sorry for the delay - Have rebased but I need to edit a few things to fix the tests, will do so asap and then we can reassess which version it will roll out in

PyCQA / docformatter