Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: astral-sh/ruff
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: astral-sh/ruff
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: brent/unary-comment2
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 8 commits
  • 3 files changed
  • 2 contributors

Commits on Nov 12, 2025

  1. Fix panic when formatting comments in unary expressions

    Summary
    --
    
    This is a second attempt at fixing #19226 based on the feedback in #20494 (comment).
    
    We currently mark the comment in an expression like this:
    
    ```py
    if '' and (not #
    0):
        pass
    ```
    
    as a leading comment on `not`, eventually causing a panic in
    `Operand::has_unparenthesized_leading_comments` because the end of the comment
    is greater than the start of the expression.
    
    https://github.com/astral-sh/ruff/blob/a1d9cb5830eca9b63e7fb529504fc536e99bca23/crates/ruff_python_formatter/src/expression/binary_like.rs#L843
    
    This PR fixes the issue by instead making such a comment a dangling comment on
    the unary expression.
    
    In the third commit, I instead tried making the comment a leading comment on the
    operand, which also looks pretty reasonable to me. Making it a dangling comment
    seems more in line with the docs on `handle_unary_op_comments`, though.
    
    I also tried deleting the leading comment logic in favor of the new dangling
    logic in the fifth commit before reverting in the sixth. This looks okay to me
    too, but the current state of the PR seems like the least invasive fix.
    
    Test Plan
    --
    
    A new, minimized test case based on the issue. I also checked that the original
    snippet from the report works now.
    
    Co-authored-by: Takayuki Maeda <[email protected]>
    ntBre and TaKO8Ki committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    923b4dd View commit details
    Browse the repository at this point in the history
  2. add failing test

    ntBre committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    6eebc35 View commit details
    Browse the repository at this point in the history
  3. leading operand comment

    ntBre committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    a303d1a View commit details
    Browse the repository at this point in the history
  4. dangling operator comment

    fix leading comments
    ntBre committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    ecccfd2 View commit details
    Browse the repository at this point in the history
  5. try only dangling

    ntBre committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    1cf5eff View commit details
    Browse the repository at this point in the history
  6. Revert "try only dangling"

    This reverts commit 4c8540ede798f544a895225b541f46552649fd7b.
    ntBre committed Nov 12, 2025
    Configuration menu
    Copy the full SHA
    f49fda3 View commit details
    Browse the repository at this point in the history

Commits on Nov 13, 2025

  1. Configuration menu
    Copy the full SHA
    9bee558 View commit details
    Browse the repository at this point in the history
  2. try a different approach

    ntBre committed Nov 13, 2025
    Configuration menu
    Copy the full SHA
    e0b0b54 View commit details
    Browse the repository at this point in the history
Loading