Skip to content

Keep the multiline layout of a sorted literal that has a trailing comment - #2686

Merged
DanielNoord merged 1 commit into
PyCQA:mainfrom
youdie006:reexports-trailing-comma-comment
Oct 1, 2026
Merged

DanielNoord merged 1 commit into
PyCQA:mainfrom
youdie006:reexports-trailing-comma-comment

Conversation

@youdie006

@youdie006 youdie006 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

#2605 and #2647 (both merged 2026-09-21) interact. #2605 keeps a multiline, trailing-comma literal in its layout when --sort-reexports runs under a trailing-comma profile, deciding with _has_trailing_comma(literal). Since #2647, literal also holds whatever follows the closing bracket, so a comment there hides the comma:

isort.code('__all__ = [\n    "b",\n    "a",\n]\n', profile="black", sort_reexports=True)
# '__all__ = [\n    "a",\n    "b",\n]\n'

isort.code('__all__ = [\n    "b",\n    "a",\n]  # noqa: F405\n', profile="black", sort_reexports=True)
# '__all__ = ["a", "b"]  # noqa: F405\n'

The trailing comma goes too, so Black does not expand it back. This moves #2647's end-position computation above the check and passes literal[:value_end]; the block and its comment are moved, not rewritten.

Verification

Base 13758cf5. New test next to #2605's in tests/unit/test_regressions.py.

row isort/literal.py md5 result
main cecdbba24c4f new test fails
this PR 147208bf4bf5 passes
literal[:value_end - 1] 1087b8373008 also fails #2605's two _issue_2578 tests and four of #2669's tests

Following continuous-integration.yml: uv sync --all-extras --frozen, then uv run --with tox-uv tox -e py,coverage_report-ci: 660 passed, 1 skipped, isort/literal.py at 100%. tox -e lint passes mypy, isort, flake8 and ruff; it reports one bandit B101 at isort/parse.py:411, identical on main, in a file this does not touch. Not run: the integration job, other Python versions, Windows and macOS.

Written with AI assistance (Claude); the measurements above were run locally and I have reviewed the change.

@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.41%. Comparing base (9c00c1f) to head (f467420).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2686   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files          41       41           
  Lines        3231     3231           
  Branches      690      690           
=======================================
  Hits         3212     3212           
  Misses         12       12           
  Partials        7        7           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

AST byte offsets are incorrectly used as character indices, potentially corrupting comments after non-ASCII literals.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes multiline literal layout preservation when a trailing comment follows the closing bracket.

Changes:

  • Detects trailing commas only within the parsed literal.
  • Adds regression coverage for commented multiline reexports.
File Description
isort/​literal.py Uses the literal boundary for trailing-comma detection.
tests/​unit/​test_regressions.py Tests multiline layout with a trailing comment.

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

Comment thread isort/literal.py
…ment

The trailing-comma check from PyCQA#2605 looked at the whole literal text, and since
the bracket hid the trailing comma, so

    __all__ = [
        "b",
        "a",
    ]  # noqa: F405

was collapsed to `__all__ = ["a", "b"]  # noqa: F405` under the black profile
with --sort-reexports. Check the trailing comma on the literal itself, using
the end position PyCQA#2647 already computes.
@DanielNoord
DanielNoord force-pushed the reexports-trailing-comma-comment branch from 8cb2fc1 to f467420 Compare October 1, 2026 19:56
@DanielNoord
DanielNoord enabled auto-merge October 1, 2026 19:56
@DanielNoord
DanielNoord added this pull request to the merge queue Oct 1, 2026
Merged via the queue into PyCQA:main with commit 61071d7 Oct 1, 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.

3 participants