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

Context extension fix #20

Merged
merged 3 commits into from
Jul 9, 2020
Merged

Context extension fix #20

merged 3 commits into from
Jul 9, 2020

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Jul 8, 2020

When git diff was replaced with difflib.SequenceMatcher in #11, I failed to also re-implement "context expansion". The equivalent of git diff -U<number-of-context-lines> is now properly implemented in diff_and_get_opcodes() again. Additionally, the number of extra context lines can no longer grow beyond the number of lines in the processed file.

What context expansion does is:

In case the AST of the original edited file doesn't match with the AST of the partially reformatted one, darker considers one more line before and after every user-edited chunk as part of the chunk. This expansion is repeated one line at a time until the ASTs match.

Context expansion helps is rare cases of two nearby chunks of Black reformatting separated by intact lines. Sometimes applying only one of those nearby chunks causes the AST to change, and context expansion will eventually cause them to be applied together.

`opcodes_to_edit_linenums()` now accepts a `context_lines` argument
which specifies how many extra lines before and after a non-equal diff
chunk should be included in the list of line numbers. This
re-introduces the inadvertently broken support for expanding the
application of reformatting around edited chunks in case the AST of
reformatted code doesn't match the original.
It didn't make sense to try to mark more surrounding lines as edited
than there are lines of text in the file.

Remove the 1000-line hard cap for the number of extra context lines.

Theoretically this could prolong the run time for very long
pathological files whose AST partial reformatting fails to maintain
unchanged. Good luck trying to come up with such a case, though.  If
this should happen, it would most probably reveal a bug in Black
instead.
`opcodes_to_edit_linenums()` now limits the last chunk to end at the
last line of the file even if the number of context lines is large.
@akaihola akaihola merged commit 4a4c423 into master Jul 9, 2020
@akaihola akaihola deleted the context-extension-fix branch July 9, 2020 11:38
@akaihola akaihola added this to the 1.0.0 milestone Jul 11, 2020
@akaihola akaihola added the bug Something isn't working label Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants