fix: make sorted __all__ and literals black-compatible (#2280)#2576
Open
lord-haffi wants to merge 6 commits into
Open
fix: make sorted __all__ and literals black-compatible (#2280)#2576lord-haffi wants to merge 6 commits into
lord-haffi wants to merge 6 commits into
Conversation
isort sorted __all__ (--sort-reexports) and inline literals (# isort: list, # isort: dict, # isort: tuple, ...) through stdlib pprint, which reads only line_length and ignores quote style, wrapping mode and trailing commas. Under --profile=black this produced single-quoted, pprint-wrapped, comma-less output that black immediately reformatted, so isort + black never reached a fixpoint. Replace the pprint path with a small black-compatible formatter in isort.literal: - black-style quoting: prefer double quotes, fall back to single only to avoid escaping, and defer to repr() for backslash/control chars and non-strings; - a single line when it fits line_length, otherwise vertical-hanging-indent with a trailing comma controlled by include_trailing_comma; - the literal's own bracket type is preserved, keeping the mandatory trailing comma that a one-element tuple needs. pprint and ISortPrettyPrinter are removed entirely; dict sorting is formatted the same way, fixing the identical black incompatibility for # isort: dict. Closes PyCQA#2280
# Conflicts: # tests/unit/test_literal.py # tests/unit/test_regressions.py
…tive (PyCQA#2280) The reexport handler wrote the `__all__` opening line to the output and then did `output_stream.seek(output_stream.tell() - reexport_rollback)` to overwrite it with the sorted literal. In check mode the output is a no-op stream (`_EmptyIO`) whose `tell()` is always 0, so a multi-line `__all__` made the seek target negative and raised `ValueError: Negative seek position`. Because the black-compatible formatter now emits multi-line `__all__`, `isort --check` crashed on isort's own output. Clamp the seek target to 0. Real seekable output is unaffected: there `tell()` is always >= the rollback length, so the clamp is a no-op.
Author
|
@Helveg Please take a look. For me it would work. I deliberately did not try to merge it with the formatting logic for the imports. In my head it made sense to keep that seperated from this logic here even though it is very similar at some parts. What are your thoughts on this? |
Author
|
I just realized, it probably addresses the same problem as #2573. |
Author
|
Claude says: Overlap with #2573Heads-up that #2573 (thanks @HarperZ9) targets the same area and also closes #1815, so these two should be reconciled. This PR also resolves #1815 and is a superset of #2573 — I ran #2573 locally against the relevant cases to compare fairly:
Root differences:
|
…LK-D209); document _repr_element
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2576 +/- ##
==========================================
+ Coverage 99.32% 99.39% +0.06%
==========================================
Files 41 41
Lines 3111 3137 +26
Branches 674 680 +6
==========================================
+ Hits 3090 3118 +28
Misses 11 11
+ Partials 10 8 -2 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: #2280, #1815
Problem
--sort-reexports(and the# isort: list/dict/tuple/…comments) format the sorted result with stdlibpprint. Under--profile=blackthe result is single-quoted, packed inpprint's wrap style and lacks a trailing comma — output black immediately reformats, so isort + black never reach a fixpoint.Fix
Format the sorted literal to be black-compatible: double quotes, a single line when it fits
line_length, otherwise one item per line (vertical-hanging-indent) with a trailing comma wheninclude_trailing_commais set, keeping the literal's own bracket type. It's a fixed black/PEP 8 layout — it deliberately does not reproduce everymulti_line_outputmode for literals (VHI is what black uses).isort's output for a long
__all__under--profile=black, before → after this fix:Why drop pprint
pprint exposes only width; quote style, wrap mode and trailing comma are hardcoded to non-black output, so it could never satisfy
--profile=black. It was the sole cause of the bug, so it's removed entirely — dict sorting now uses the same formatter and gets the same fix.Also fixes a
--checkcrash on multi-line__all__Emitting multi-line
__all__exposed a latent bug in the--sort-reexportsrollback: it wrote the__all__opening line then didoutput_stream.seek(output_stream.tell() - reexport_rollback)to overwrite it with the sorted literal. In check mode the output is a no-op stream whosetell()is always0, so the seek went to a negative position and raisedValueError: Negative seek position. The seek target is now clamped to0; real seekable output is unaffected.Note: sorted string literals are now double-quoted by default (isort has no quote setting); existing literal-sort test expectations are updated to match.