Preserve LF stdin output on Windows

[AleksZyro]

Refs #2453.

When sorting stdin to stdout on Windows, Python's text-mode stdout can translate \n to \r\n. That makes LF input become CRLF output even when isort inferred LF from the input stream.

This writes stdin sorting output through a stdout wrapper with newline translation disabled, and adds a regression test for LF input written through a text stdout stream.

Tested with:

python -m pytest tests/unit/test_main.py::test_isort_with_stdin_preserves_lf_stdout -q
python -m pytest tests/unit/test_main.py::test_isort_with_stdin -q
python -m ruff check isort/main.py tests/unit/test_main.py
python -m ruff format --check isort/main.py tests/unit/test_main.py

[Copilot]

Pull request overview

Preserves LF line endings when sorting stdin to stdout on Windows.

Changes:

• Adds a no-newline-translation stdout wrapper.
• Adds an LF preservation regression test.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
isort/main.py	Routes stdin sorting output through the wrapper.
tests/unit/test_main.py	Tests LF output preservation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[DanielNoord]

Please check if this comment makes sense (or whether it is a hallunication) and if it isn't add test cases for it.

Sorry, I'm quite busy these days so trying to leverage AI to speed up reviews. I acknowledge it might not always produce accurate reviews.

[AleksZyro]

Thanks, I checked both comments and they make sense.

I rebased onto current main, narrowed the fallback so it only catches wrapper construction failures, and also wrapped stdin with newline translation disabled. I added a CRLF regression test alongside the LF case.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.37%. Comparing base (d5f40fa) to head (58270f6).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2616   +/-   ##
=======================================
  Coverage   99.36%   99.37%           
=======================================
  Files          41       41           
  Lines        3165     3188   +23     
  Branches      682      686    +4     
=======================================
+ Hits         3145     3168   +23     
  Misses         12       12           
  Partials        8        8           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DanielNoord]

My rebase uncovered some typing issues which break the mypyc build. Could you look into that?

[AleksZyro]

Thanks, I fixed the typing issue in the stream wrapper and added coverage for the non-TextIOWrapper fallback path.

Locally checked:

• mypy on isort/main.py and tests/unit/test_main.py
• the affected stdin newline tests
• tests/unit/test_main.py
• ruff check/format on the changed files

I also tried the mypyc wheel build locally; it now gets past the typing step and only stops because this machine is missing Microsoft C++ Build Tools.

[DanielNoord]

The new abstraction seems a bit awkward. Wasn't isort/main.py:77: error: Incompatible types in "yield" (actual type "IO[Any]", expected type "TextIO") [misc] fixable by updating the type annotations?

[AleksZyro]

Yes, that was fair. I simplified this to avoid the extra context manager class: the helper now returns either a nullcontext for the original stream or the typed open() context manager directly.

The type issue is handled with the narrower return annotation/cast, while keeping the fallback scoped to opening the wrapper.

[DanielNoord]

Sorry, did a very quick review but need to check out this branch locally and do some more testing. For now I have a simple question. Ideally this would be fixed without casts, but I guess that can't work? Or can it?

[DanielNoord]

Made the code and intentions a bit more explicit, that also removes the need for the cast.

I don't really like catching the OSError and opening the stream though. Idea I just had: what if we just check if we are dealing with sys.stdin and sys.stdout instead and only do the open and special handling in that case?

[AleksZyro]

That does make sense; by limiting the special treatment to sys.stdin and sys.stdout the custom streams will remain unaltered and unrelated stream errors will be avoided. I'll look at the resulting behaviour and the regression coverage before carrying out another update.