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

Format docstrings as complete chunks #300

Merged
merged 6 commits into from
Apr 5, 2022
Merged

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Feb 18, 2022

Treating large docstrings as separate chunks can cause to weird behavior, e.g. "reformat loops" in which some lines are repeatedly indented and unindented by Darker.

Fixes #240.

  • add failing test case
  • find multi-line using AST parsing, using e.g.:
    from pathlib import Path
    from tokenize import tokenize, STRING
    from darker.utils import TextDocument
    
    src = TextDocument.from_file(Path("setup.py"))
    readline = (f"{line}\n".encode(src.encoding) for line in src.lines).__next__
    multiline_string_linespans = [
        (t.start[0], t.end[0]) for t in tokenize(readline)
        if t.type == STRING and t.end[0] > t.start[0]
    ]
  • if any line in a multi-line string was edited by the user, mark all of the lines edited
  • change log
  • tests for added functions

@akaihola akaihola added the bug Something isn't working label Feb 18, 2022
@akaihola akaihola added this to the 1.4.2 milestone Feb 18, 2022
@akaihola akaihola requested a review from Carreau February 18, 2022 21:20
@akaihola akaihola self-assigned this Feb 18, 2022
@akaihola akaihola marked this pull request as draft February 18, 2022 21:20
@akaihola akaihola added the help wanted Extra attention is needed label Feb 19, 2022
@akaihola
Copy link
Owner Author

akaihola commented Feb 19, 2022

@Carreau, any other ideas for fixing this than doing our own AST parsing to find where possible multi-line docstrings are?

Update: tokenize.tokenize() was sufficient, no need for full AST parsing. I think this is a reasonable solution, what's your opinion?

@akaihola akaihola force-pushed the docstring-indent-loop branch from 4176994 to a173a02 Compare February 21, 2022 13:07
@akaihola akaihola force-pushed the docstring-indent-loop branch 2 times, most recently from 57ebd0e to 1189751 Compare February 21, 2022 17:53
@akaihola akaihola marked this pull request as ready for review February 21, 2022 18:09
@akaihola akaihola force-pushed the docstring-indent-loop branch 6 times, most recently from 2f583c2 to cab967d Compare February 27, 2022 18:45
@akaihola
Copy link
Owner Author

@Carreau, this seems to work now, and I found two edge case bugs in my implementation when completing the unit test coverage! I wonder if a second pair of eyes reviewing this would find even more.

I could include this in version 1.4.2 if you have time to review, or alternatively postpone until the 1.5.0 release.

src/darker/tests/test_multiline_strings.py Show resolved Hide resolved
src/darker/tests/test_multiline_strings.py Show resolved Hide resolved
src/darker/tests/test_diff.py Show resolved Hide resolved
src/darker/tests/test_diff.py Show resolved Hide resolved
src/darker/tests/test_git.py Show resolved Hide resolved
src/darker/tests/test_git.py Show resolved Hide resolved
src/darker/tests/test_main_reformat_single_file.py Outdated Show resolved Hide resolved
@akaihola akaihola force-pushed the docstring-indent-loop branch from 65c48ab to 0257656 Compare March 13, 2022 19:57
@akaihola
Copy link
Owner Author

Moving to version 1.5.0 (1.4.2 was just released).

@akaihola akaihola modified the milestones: 1.4.2, 1.5.0 Mar 13, 2022
@akaihola akaihola force-pushed the docstring-indent-loop branch 3 times, most recently from d1a1a77 to 614f134 Compare March 14, 2022 17:52
@akaihola akaihola force-pushed the docstring-indent-loop branch 3 times, most recently from de72ab0 to 4a59878 Compare March 26, 2022 21:43
@akaihola akaihola force-pushed the docstring-indent-loop branch 3 times, most recently from 35e6413 to 32613c4 Compare April 1, 2022 19:43
@akaihola akaihola force-pushed the docstring-indent-loop branch from 32613c4 to 0f9a8a2 Compare April 5, 2022 16:12
@akaihola akaihola merged commit 1e742d7 into master Apr 5, 2022
@akaihola akaihola deleted the docstring-indent-loop branch April 5, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
Development

Successfully merging this pull request may close these issues.

Darker keeps indenting docstrings
2 participants