black
black copied to clipboard
Fix for #4264: --line-ranges formats entire file when ranges are at EOF
Description
This fixes #4264. The issue was that the empty last line does not count as a line to adjusted_lines
because it is not in the list returned by str.splitlines
. Since adjusted_lines
is only called on the second pass of _format_str_once
in format_str
, the first pass would format the code correctly, then the "invalid" line would get removed from lines
, and the second pass would format the whole code.
I added a small change to adjusted_lines
to cap the end value of any line tuple. For example, --line-ranges 1-100
gets reduced to (1, 4)
if the code is only four lines long.
One could change if end > original_line_count
to if end == original_line_count + 1
to only allow this one additional line, but I think allowing an oversized range to just cover the rest of the code is not surprising behavior, slices act similarly.
I also added a call to adjusted_lines
with the unmodified source code before the first pass of _format_str_once
. This is an additional computational expense but ensures consistency.
Checklist - did you ...
- [x] Add an entry in
CHANGES.md
if necessary? - [x] Add / update tests if necessary?
- [-] Add new / update outdated documentation?
diff-shades reports zero changes comparing this PR (a1ba8778a2c33f051cb66af2468cb8621a1a86c9) to main (1abcffc81816257985678f08c61584ed4287f22a).
cc @yilei
Thanks for tagging, @JelleZijlstra!
I also added a call to adjusted_lines with the unmodified source code before the first pass of _format_str_once. This is an additional computational expense but ensures consistency.
Why is this still necessary, after the change to adjusted_lines to cap the end value?
Could you also add a test file next to https://github.com/psf/black/blob/main/tests/data/cases/line_ranges_basic.py ?
This opens up a question on what happens when you specify a --line-ranges=
that's outside of the unformatted file. This change makes it valid when you specify a larger <END>
, but if the entire range is outside then the entire file is still formatted. How about make it format nothing if everything is outside of the range too? The implementation could be, instead of changing adjusted_lines
, to call a new sanitize_lines(lines, src_contents)
once in format_str
:
def format_str(...):
if lines:
lines = sanitize_lines(lines, src_contents)
if not lines:
return src_contents # Nothing to format
dst_contents = _format_str_once(src_contents, mode=mode, lines=lines)
...
Thank you so much for the feedback!
Calling adjusted_lines
beforehand is not really necessary. I just used it like one would sanitize_lines
to make sure that any lines that would be removed on the second pass would already be removed on the first. Writing a new function for that definitely makes more sense!
Thank you also for noticing that moving the entire range out of the file still formats everything, I'll fix that as suggested.
Thanks for reviewing and approving my PR! Glad to be able to contribute to one of my all-time favorite Python projects :)