Visitar URL original
Preserve file line endings when sorting reexports by vyrnsynx · Pull Request #2669 · PyCQA/isort · GitHub
Skip to content

Preserve file line endings when sorting reexports - #2669

Merged
DanielNoord merged 4 commits into
PyCQA:mainfrom
vyrnsynx:fix/sort-reexports-line-endings
Sep 25, 2026
Merged

DanielNoord merged 4 commits into
PyCQA:mainfrom
vyrnsynx:fix/sort-reexports-line-endings

Conversation

@vyrnsynx

@vyrnsynx vyrnsynx commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

--sort-reexports was rebuilding __all__ / sorted literals with hardcoded \n, so a CRLF file ended up with mixed line endings after a sort (same path as # isort: list and friends).

Fixes #2668.

Change

In isort/literal.py, the hanging-indent formatter now uses the active line separator: config.line_ending when set, otherwise whatever the source text already uses (\r\n / \r / \n), matching how the rest of isort picks endings.

Tests

  • CRLF tuple- and list-style __all__ under --sort-reexports (black profile)
  • CRLF # isort: list action-comment path
  • Direct literal API coverage for line_ending config and CRLF inference

pytest tests/unit/test_literal.py tests/unit/test_regressions.py → 135 passed

Hardcoded LF in the literal formatter mixed endings in CRLF files
under --sort-reexports. Honor config.line_ending, otherwise match
the source text the same way the rest of isort does.
@vyrnsynx
vyrnsynx marked this pull request as ready for review September 23, 2026 01:30
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.44%. Comparing base (f00e091) to head (af081ad).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2669   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files          41       41           
  Lines        3222     3225    +3     
  Branches      692      692           
=======================================
+ Hits         3204     3207    +3     
  Misses         11       11           
  Partials        7        7           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DanielNoord DanielNoord left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use _infer_line_separator instead of adding the same utility twice. Potentially by replacing the places where we call _infer_line_separator with your more ergonomic version :)

Also, please fix coverage :)

Drop the duplicate helper in literal.py and honor config.line_ending
the same way file_contents does, so CRLF handling stays in one place.
@vyrnsynx

Copy link
Copy Markdown
Contributor Author

Thanks for the review — agreed.

I dropped the duplicate helper and now reuse parse._infer_line_separator, with the same config.line_ending or … pattern file_contents already uses. That also removes the uncovered \r-only branch from the PR diff (coverage lives with the existing parse helper / fuzz test).

@DanielNoord DanielNoord left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@DanielNoord
DanielNoord added this pull request to the merge queue Sep 25, 2026
Merged via the queue into PyCQA:main with commit 822b695 Sep 25, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--sort-reexports hardcodes LF line endings regardless of platform

2 participants