google / google/yapf

indent_dictionary_value:True breaks lines even when values fit on the same line

Open
#547 8 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
14k
Forks
904
PR merge metrics
No merged PRs in 30d

Description

The knob documentations says `indent_dictionary_value:True` will "indent the dictionary value _if it cannot fit on the same line as the dictionary key"_. However I see cases where this option creates terrible formatting even if every line of my input is already less than `column_limit`.

Stranger still, this new (bad) splits are not even between dictionary key and value, as the knob would have suggested.

**nested-list.py**
```python
_schema = [
('uuid', {'field_type': 'STRING'}),
('ingredient_line', {'field_type': 'STRING'}),
('item_name', {'field_type': 'STRING'}),
('query', {'field_type': 'STRING'}),
('item_name_seq', {'field_type': 'INTEGER', 'mode': 'REPEATED'}),
('query_seq', {'field_type': 'INTEGER', 'mode': 'REPEATED'}),
('relevance', {'field_type': 'FLOAT'}),
('fold', {'field_type': 'INTEGER'}),
('params', {'field_type': 'STRING'}),
('timestamp', {'field_type': 'DATETIME'})
]
```

**To reproduce**
```
yapf --style '{indent_dictionary_value:True}' --diff nested-list.py
--- nested-list.py (original)
+++ nested-list.py (reformatted)
@@ -1,12 +1,23 @@
-_schema = [
- ('uuid', {'field_type': 'STRING'}),
- ('ingredient_line', {'field_type': 'STRING'}),
- ('item_name', {'field_type': 'STRING'}),
- ('query', {'field_type': 'STRING'}),
- ('item_name_seq', {'field_type': 'INTEGER', 'mode': 'REPEATED'}),
- ('query_seq', {'field_type': 'INTEGER', 'mode': 'REPEATED'}),
- ('relevance', {'field_type': 'FLOAT'}),
- ('fold', {'field_type': 'INTEGER'}),
- ('params', {'field_type': 'STRING'}),
- ('timestamp', {'field_type': 'DATETIME'})
-]
+_schema = [('uuid', {
+ 'field_type': 'STRING'
+}), ('ingredient_line', {
+ 'field_type': 'STRING'
+}), ('item_name', {
+ 'field_type': 'STRING'
+}), ('query', {
+ 'field_type': 'STRING'
+}), ('item_name_seq', {
+ 'field_type': 'INTEGER',
+ 'mode': 'REPEATED'
+}), ('query_seq', {
+ 'field_type': 'INTEGER',
+ 'mode': 'REPEATED'
+}), ('relevance', {
+ 'field_type': 'FLOAT'
+}), ('fold', {
+ 'field_type': 'INTEGER'
+}), ('params', {
+ 'field_type': 'STRING'
+}), ('timestamp', {
+ 'field_type': 'DATETIME'
+})]
```

My expectation:
- [ ] `indent_dictionary_value:True` does not reformat lines that already fit on one line
- [ ] `indent_dictionary_value:True` does not introduce new splits that aren't between dict key and value.

Note that if I keep the default of `indent_dictionary_value:True`, the yapf (correctly) does not reformat my code. (My motivation for forcing `indent_dictionary_value=True` is to workaround #392.)

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.