Keep long comments off the opening line and within line_length

[SemTiOne]

• Resolves Line Length Ignores Comments #2124

vertical_hanging_indent now renders each comment fragment on its own # line instead of merging them onto the from ... import ( line with "; ".

[Manny7717]

Verified locally on head 67213aa (tests/unit: 627 passed / 2 failed / 1 skipped — the 2 failures are the known pre-existing FileNotFoundError env noise (issue_909/938), byte-identical on main; main baseline is 623 passed, so the +4 are exactly this PR's new tests, zero regressions).

Fix behavior confirmed:

• Issue #2124 shape (black profile, long comment inside a multi-line from-import): the comment now moves to its own line under the opening paren instead of being merged onto from ... import ( — isort.code output verified, and the result is idempotent (re-running produces identical output).
• Functional comments are correctly exempt: # noqa: F401 and # type: ignore stay on the opening line (verified end-to-end — moving them would break their meaning).
• Short comments still stay on the opening line (existing behavior preserved, covered by test_vertical_hanging_indent_short_comment_stays_on_opening_line).
• wrap.import_statement(explode=True) now passes the real line_length instead of 1; checked the vertical_hanging_indent interface — line_length is only consumed by the new branch, so no other explode-mode behavior changes.
• isort self-lint (--profile hug --check --diff isort/ tests/) and ruff on the changed files: clean.

Non-blocking nit: the functional-comment detection uses a loose substring ("noqa" in text or "type:" in text). A comment that merely mentions these tokens (e.g. # check type: int ...) is treated as functional and stays on the over-long opening line. This errs in the safe direction (it's the old behavior), so not a blocker — but anchoring the check to a directive-shaped pattern (e.g. comment starting with # noqa / # type: ignore) would make it tighter. No change requested.

[Manny7717]

Re-verified on 9683ad6 — my wrap_modes.py:178 nit is fully addressed.

Verification (local):

1. Regression proven: test_vertical_hanging_indent_non_directive_type_comment_moves_off FAILS on 67213aa (old substring "type:" in _comment_text kept a check type: int comment on the opening line) and PASSES on 9683ad6. Exactly the false-positive I flagged.
2. Functional exemptions intact (executed probes): long noqa: F401 ... comment stays on the opening line (from os.path import ( # noqa: ...); long type: ignore ... stays as well. Non-directive check type: int and plain long comments move off to their own line under the paren.
3. No regressions: full unit suite on head = 628 passed / 2 failed — the 2 failures (issue_909/938 FileNotFoundError) are the documented pre-existing env noise, byte-identical on base. Base = 627 passed / 3 failed (env noise + the new regression test). Zero new failures.

The comment.strip().lower().startswith(("noqa", "type: ignore")) per-comment check is a strictly better heuristic than the substring match, and the test pins the non-directive case so it can't regress.

[DanielNoord]

After seeing the code I am not sure that this is a bug. Not sorting the imports also isn't the correct behaviour. Perhaps there is just no solution for this?

[SemTiOne]

Isn't issue #2124 about the merged comment violating line_length? The user had two separate comment lines that got collapsed onto from ... import ( and blew past 88 chars.

With the fix:

from os.path import (
    # this is a really really really really really really really really
    # really really really really really really long comment
    getsize,
    join,
)

--- line lengths ---
1:  21 | from os.path import (
2:  71 |     # this is a really really really really really really really really
3:  60 |     # really really really really really really long comment
4:  12 |     getsize,
5:   9 |     join,
6:   1 | )
--- max: 71 ---
--- idempotent: True ---

[Manny7717]

Re-approving on the new head 94104cf ("Render each comment fragment on its own line"). This commit answers the maintainer's concern that a long merged comment could still violate line_length: each comment fragment now renders on its own # line under the paren via line_separator.join(add_to_line([fragment], ...)).

Verified locally on 94104cf:

• New test test_vertical_hanging_indent_multi_fragment_comments_each_on_own_line passes; every output line asserted <= 88 chars.
• End-to-end: two long comment fragments under VHI (multi_line_output=3, line_length=88) render on separate indented lines, imports sorted, idempotent (re-run byte-identical).
• Short comments unchanged (stay on opening line when they fit); functional comments (noqa/type: ignore) still exempt.
• Full unit suite: 629 passed / 2 failed (issue_909/938 FileNotFoundError env noise, identical on main) / 1 skipped - zero regressions, exactly +1 test.

Note on the maintainer thread: per-fragment rendering is the right resolution - the previous behavior (all fragments joined onto one line) could still blow past line_length, which is the actual #2124 complaint. Approved.

[SemTiOne]

After looking at it again, I found some nits and edge cases. I'll fix them soon.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (5a6aaf5) to head (16681a8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2645   +/-   ##
=======================================
  Coverage   99.37%   99.38%           
=======================================
  Files          40       40           
  Lines        3224     3228    +4     
  Branches      689      690    +1     
=======================================
+ Hits         3204     3208    +4     
  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]

This seems to be a breaking change for one of the projects in the primer. Are we sure we are on the right track here? I don't expect this to require breaking changes, or, put differently, don't think this is important enough to fix to warrant breaking changes.

[SemTiOne]

This seems to be a breaking change for one of the projects in the primer. Are we sure we are on the right track here? I don't expect this to require breaking changes, or, put differently, don't think this is important enough to fix to warrant breaking changes.

Oop, my bad. I accidentally changed the existing primer behavior, and this fix is way too broad. Let me fix it.

[SemTiOne]

parse.py:306 stores these opening comments in the __combined_as__ bucket, while output.py:766 pops them back without preserving opening provenance. The current opening recording only covers the plain from bucket.

Could this be a follow-up issue? @DanielNoord

[Copilot]

Copilot review overview

🟢 Approved

The implementation addresses the reported formatting regression while preserving opening-line and functional comments with comprehensive tests.

Review effort: Balanced
Findings: None

What changed in this PR

This PR resolves #2124 by preserving comment fragments on separate lines when merged comments would exceed the configured line length.

Changes:

• Tracks opening-line comment provenance through parsing and output.
• Moves long body-comment fragments onto individual indented lines.
• Adds unit and regression coverage for formatting, directives, and idempotency.

File	Description
isort/​parse.py	Records opening-line comments separately.
isort/​output.py	Propagates opening-comment metadata during formatting.
isort/​wrap.py	Passes comment provenance and configured line length to wrappers.
isort/​wrap_modes.py	Separates movable comment fragments from preserved comments.
tests/​unit/​test_wrap.py	Tests exploded import formatting.
tests/​unit/​test_wrap_modes.py	Tests comment placement across relevant cases.
tests/​unit/​test_regressions.py	Adds end-to-end regression coverage for #2124.

---

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

[Copilot]

🟡 Changes recommended

Comment-count gating cannot reliably preserve opening-line comments or handle single-body-comment overflow.

1 open issue

• Preserve comment origins when wrapping imports · New

_{Review effort: Balanced}