google / google/yapf

SPLIT_ALL_COMMA_SEPARATED_VALUES should be recursive/nested for parameters

Open
#574 1 comment 11 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
14k
Forks
904
PR merge metrics
No merged PRs in 30d

Description

Using yapf 0.22

Love the addition of `SPLIT_ALL_COMMA_SEPARATED_VALUES`

> If a comma separated list (dict, list, tuple, or function def) is on a line that is too long, split such that all elements are on a single line.

(I never got used to pep8's cram-as-much-in-per-line-as-you-can style... not very readable in my experience)

However, when `SPLIT_ALL_COMMA_SEPARATED_VALUES` triggers a split, it splits all parameters at *all* levels. Meaning, if a parameter is itself a function that can all fit on the same line, it too gets split.

For example, consider PEP8 code like this:

```python
# Baseline: PEP-8

def make_table():
create_table('users', Column('id', Integer(), nullable=False),
Column('email', String(length=50), nullable=False),
Column('password', String(length=60), nullable=False),
Column('first_name', String()), Column('last_name', String()),
Column('last_login', DateTime(), nullable=True),
PrimaryKeyConstraint('id'))
```

(note how the `create_table` and third-to-last lines have multiple items on it -- hard to read, this is why I like the split option!)

yapf 0.22 with `SPLIT_ALL_COMMA_SEPARATED_VALUES = true` transforms it to this:

```python
# yapf with SPLIT_ALL_COMMA_SEPARATED_VALUES

def make_table():
create_table('users',
Column('id',
Integer(),
nullable=False),
Column('email',
String(length=50),
nullable=False),
Column('password',
String(length=60),
nullable=False),
Column('first_name',
String()),
Column('last_name',
String()),
Column('last_login',
DateTime(),
nullable=True),
PrimaryKeyConstraint('id'))
```

But that now is overly verbose. All those `Column(...)` params COULD fit on one line, so I would expect those nested/2nd-level parameters NOT to be split:

```python
# Expected

def make_table():
create_table('users',
Column('id', Integer(), nullable=False),
Column('email', String(length=50), nullable=False),
Column('password', String(length=60), nullable=False),
Column('first_name', String()),
Column('last_name', String()),
Column('last_login', DateTime(), nullable=True),
PrimaryKeyConstraint('id'))
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.