Skip to content

Commit

Permalink
Break global and nonlocal statements over continuation lines (#6172)
Browse files Browse the repository at this point in the history
## Summary

Builds on #6170 to break `global` and `nonlocal` statements, such that
we get:

```python
def f():
    global \
        analyze_featuremap_layer, \
        analyze_featuremapcompression_layer, \
        analyze_latencies_post, \
        analyze_motions_layer, \
        analyze_size_model
```

Instead of:

```python
def f():
    global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model
```

Notably, we avoid applying this formatting if the statement ends in a
comment. Otherwise, the comment would _need_ to be placed after the last
item, like:

```python
def f():
    global \
        analyze_featuremap_layer, \
        analyze_featuremapcompression_layer, \
        analyze_latencies_post, \
        analyze_motions_layer, \
        analyze_size_model  # noqa
```

To me, this seems wrong (and would break the `# noqa` comment). Ideally,
the items would be parenthesized, and the comment would be on the inner
parenthesis, like:

```python
def f():
    global (  # noqa
        analyze_featuremap_layer,
        analyze_featuremapcompression_layer,
        analyze_latencies_post,
        analyze_motions_layer,
        analyze_size_model
    )
```

But that's not valid syntax.
  • Loading branch information
charliermarsh committed Aug 2, 2023
1 parent 9f38dbd commit 9425ed7
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ def f():

def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model


def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@ def f():

def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model


def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
43 changes: 39 additions & 4 deletions crates/ruff_python_formatter/src/statement/stmt_global.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_formatter::{format_args, write};
use ruff_python_ast::node::AstNode;
use ruff_python_ast::StmtGlobal;

use crate::prelude::*;
Expand All @@ -9,10 +10,44 @@ pub struct FormatStmtGlobal;

impl FormatNodeRule<StmtGlobal> for FormatStmtGlobal {
fn fmt_fields(&self, item: &StmtGlobal, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [text("global"), space()])?;
// Join the `global` names, breaking across continuation lines if necessary, unless the
// `global` statement has a trailing comment, in which case, breaking the names would
// move the comment "off" of the `global` statement.
if f.context()
.comments()
.has_trailing_comments(item.as_any_node_ref())
{
let joined = format_with(|f| {
f.join_with(format_args![text(","), space()])
.entries(item.names.iter().formatted())
.finish()
});

f.join_with(format_args![text(","), space()])
.entries(item.names.iter().formatted())
.finish()
write!(f, [text("global"), space(), &joined])
} else {
let joined = format_with(|f| {
f.join_with(&format_args![
text(","),
space(),
if_group_breaks(&text("\\")),
soft_line_break(),
])
.entries(item.names.iter().formatted())
.finish()
});

write!(
f,
[
text("global"),
space(),
&group(&format_args!(
if_group_breaks(&text("\\")),
soft_line_break(),
&soft_block_indent(&joined)
))
]
)
}
}
}
43 changes: 39 additions & 4 deletions crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ruff_formatter::{format_args, write};
use ruff_python_ast::node::AstNode;
use ruff_python_ast::StmtNonlocal;

use crate::prelude::*;
Expand All @@ -9,10 +10,44 @@ pub struct FormatStmtNonlocal;

impl FormatNodeRule<StmtNonlocal> for FormatStmtNonlocal {
fn fmt_fields(&self, item: &StmtNonlocal, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [text("nonlocal"), space()])?;
// Join the `nonlocal` names, breaking across continuation lines if necessary, unless the
// `nonlocal` statement has a trailing comment, in which case, breaking the names would
// move the comment "off" of the `nonlocal` statement.
if f.context()
.comments()
.has_trailing_comments(item.as_any_node_ref())
{
let joined = format_with(|f| {
f.join_with(format_args![text(","), space()])
.entries(item.names.iter().formatted())
.finish()
});

f.join_with(format_args![text(","), space()])
.entries(item.names.iter().formatted())
.finish()
write!(f, [text("nonlocal"), space(), &joined])
} else {
let joined = format_with(|f| {
f.join_with(&format_args![
text(","),
space(),
if_group_breaks(&text("\\")),
soft_line_break(),
])
.entries(item.names.iter().formatted())
.finish()
});

write!(
f,
[
text("nonlocal"),
space(),
&group(&format_args!(
if_group_breaks(&text("\\")),
soft_line_break(),
&soft_block_indent(&joined)
))
]
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def f():
def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model
def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
```

## Output
Expand All @@ -31,7 +35,16 @@ def f():
def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model
global \
analyze_featuremap_layer, \
analyze_featuremapcompression_layer, \
analyze_latencies_post, \
analyze_motions_layer, \
analyze_size_model
def f():
global analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
```


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def f():
def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model
def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
```

## Output
Expand All @@ -31,7 +35,16 @@ def f():
def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model
nonlocal \
analyze_featuremap_layer, \
analyze_featuremapcompression_layer, \
analyze_latencies_post, \
analyze_motions_layer, \
analyze_size_model
def f():
nonlocal analyze_featuremap_layer, analyze_featuremapcompression_layer, analyze_latencies_post, analyze_motions_layer, analyze_size_model # end-of-line comment
```


Expand Down

0 comments on commit 9425ed7

Please sign in to comment.