From d4cfe1f2a72e069edb03697a8186a1604506fe00 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 13 Sep 2023 12:01:55 +0200 Subject: [PATCH] Break before slice colon **Summary** Break slices at the colon first, since the colon is separator with the lowest precedence and we're in a parenthesized context. **Input** ```python section_header_data = byte_array[byte_begin_index + byte_step_index * event_index : byte_begin_index + byte_step_index * (event_index + 1)] ``` **Black** ```python section_header_data = byte_array[ byte_begin_index + byte_step_index * event_index : byte_begin_index + byte_step_index * (event_index + 1) ] ``` **Current formatting** ```python section_header_data = byte_array[ byte_begin_index + byte_step_index * event_index : byte_begin_index + byte_step_index * (event_index + 1) ] ``` **Proposed formatting** ```python section_header_data = byte_array[ byte_begin_index + byte_step_index * event_index : byte_begin_index + byte_step_index * (event_index + 1) ] ``` This is another intentional black deviation, but i find it a clear style improvement. This is consistent with adding a step: ```python section_header_data2 = byte_array[ byte_begin_index + byte_step_index * event_index : byte_begin_index + byte_step_index : section_size ] ``` As-is, this regresses trailing colon comments: **in** ```python c1 = "c"[ 1: # e # f 2 ] ``` **out** ```python c1 = "c"[ 1 : # e # f 2 ] ``` Fixes #7316 **Test Plan** Added the fixtures above. --- .../test/fixtures/ruff/expression/slice.py | 15 +++++++ .../src/expression/expr_slice.rs | 4 +- .../format@expression__slice.py.snap | 42 ++++++++++++++++--- .../snapshots/format@statement__raise.py.snap | 3 +- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/slice.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/slice.py index 3b30f67cd49bc..c8921aa913717 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/slice.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/slice.py @@ -91,3 +91,18 @@ def a(): g1 = "g"[(1):(2)] g2 = "g"[(1):(2):(3)] +# https://github.com/astral-sh/ruff/issues/7316 +section_header_data = byte_array[ + byte_begin_index + + byte_step_index * event_index : byte_begin_index + + byte_step_index * (event_index + 1) +] + +section_header_data2 = byte_array[ + byte_begin_index + + byte_step_index * event_index : byte_begin_index + + byte_step_index : section_size +] + + + diff --git a/crates/ruff_python_formatter/src/expression/expr_slice.rs b/crates/ruff_python_formatter/src/expression/expr_slice.rs index c8177e9f8019a..093ae9654d2c5 100644 --- a/crates/ruff_python_formatter/src/expression/expr_slice.rs +++ b/crates/ruff_python_formatter/src/expression/expr_slice.rs @@ -91,7 +91,7 @@ impl FormatNodeRule for FormatExprSlice { if !all_simple && lower.is_some() { space().fmt(f)?; } - token(":").fmt(f)?; + write!(f, [soft_line_break(), token(":")])?; // No upper node, no need for a space, e.g. `x[a() :]` if !all_simple && upper.is_some() { space().fmt(f)?; @@ -125,7 +125,7 @@ impl FormatNodeRule for FormatExprSlice { if !all_simple && (upper.is_some() || step.is_none()) { space().fmt(f)?; } - token(":").fmt(f)?; + write!(f, [soft_line_break(), token(":")])?; // No step node, no need for a space if !all_simple && step.is_some() { space().fmt(f)?; diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__slice.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__slice.py.snap index 482cd452c1d02..9d7fee4f51d3f 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__slice.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__slice.py.snap @@ -97,6 +97,21 @@ f = "f"[:,] g1 = "g"[(1):(2)] g2 = "g"[(1):(2):(3)] +# https://github.com/astral-sh/ruff/issues/7316 +section_header_data = byte_array[ + byte_begin_index + + byte_step_index * event_index : byte_begin_index + + byte_step_index * (event_index + 1) +] + +section_header_data2 = byte_array[ + byte_begin_index + + byte_step_index * event_index : byte_begin_index + + byte_step_index : section_size +] + + + ``` ## Output @@ -132,21 +147,25 @@ b1 = "b"[ # a # Handle the spacing from the colon correctly with upper leading comments c1 = "c"[ - 1: # e + 1 + : # e # f 2 ] c2 = "c"[ - 1: # e + 1 + : # e 2 ] c3 = "c"[ - 1: + 1 + : # f 2 ] c4 = "c"[ - 1: # f + 1 + : # f 2 ] @@ -155,7 +174,8 @@ d1 = "d"[ # comment : ] d2 = "d"[ # comment - 1: + 1 + : ] d3 = "d"[ 1 # comment @@ -191,6 +211,18 @@ f = "f"[:,] # Regression test for https://github.com/astral-sh/ruff/issues/5733 g1 = "g"[(1):(2)] g2 = "g"[(1):(2):(3)] + +# https://github.com/astral-sh/ruff/issues/7316 +section_header_data = byte_array[ + byte_begin_index + byte_step_index * event_index + : byte_begin_index + byte_step_index * (event_index + 1) +] + +section_header_data2 = byte_array[ + byte_begin_index + byte_step_index * event_index + : byte_begin_index + byte_step_index + : section_size +] ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__raise.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__raise.py.snap index 46637a35435a4..c4a14df0ff6d8 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__raise.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__raise.py.snap @@ -175,7 +175,8 @@ raise ( # hey 2 # some comment raise aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa[ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbbbbbbb + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + :bbbbbbbbbbbbbbbbbbbbbbbbbb ] raise (