split_all_top_level_comma_separated_values makes force_multiline_dict ineffective

[AndydeCleyre]

$ yapf --version
yapf 0.30.0
$ cat .style.yapf

[style]
each_dict_entry_on_separate_line=True
force_multiline_dict=True
split_all_top_level_comma_separated_values=True

$ cat test.py

COLOR_GROUPS = {
    'Meta-switches': magenta,
    'Switches': yellow,
    'Subcommands': blue
}

$ yapf --style .style.yapf -d test.py

--- test.py	(original)
+++ test.py	(reformatted)
@@ -1,5 +1,3 @@
 COLOR_GROUPS = {
-    'Meta-switches': magenta,
-    'Switches': yellow,
-    'Subcommands': blue
+    'Meta-switches': magenta, 'Switches': yellow, 'Subcommands': blue
 }

[kliyes]

You mean split_all_comma_separated_values or split_all_top_level_comma_separated_values?

[AndydeCleyre]

Whoops, thanks: split_all_top_level_comma_separated_values. I'll update the title.

[AndydeCleyre]

@brianmego Can you reproduce?

[AndydeCleyre]

I've pinpointed the bad behavior at yapf/yapflib/format_decision_state.py#L201-L204, FormatDecisionState.MustSplit:

# Allow the fallthrough code to handle the closing bracket.
if current != opening.matching_bracket:
  # If the container doesn't fit in the current line, must split
  return not self._ContainerFitsOnStartLine(opening)

Which was formerly just:

# If the container doesn't fit in the current line, must split
return not self._ContainerFitsOnStartLine(opening)

Which is problematic in the same way. 5de9ca6e546acb1730b86e982876e9cbe827bc89

[brianmego]

Well, I can reproduce this, though I'm not sure what we want to do about it. These two flags are sort of at odds with each other, I think. One says, "Split under all conditions" while the other says "Don't split unless the line is super long". Proof of the issue. This test fails:

https://github.com/brianmego/yapf/commit/52442c1adb7ee2b90471888d2e403bcd47c7143f

[AndydeCleyre]

I would think the force multiline dict knob should win, since its target is more specific, and because it's a "force" knob. I would think the other multi line dict knob should win, because its target is specific.

It feels like I'm saying, "split x, definitely split x, and also split x in some specific cases," and hearing back "no problem we found a split x and unsplit it for you."

[AndydeCleyre]

I still think that if the config includes

each_dict_entry_on_separate_line=True
force_multiline_dict=True

then dict entries ought to be placed on separate lines.

[spaceone]

This issue hit me too and blocks me from using yapf.

I think with each_dict_entry_on_separate_line=True but force_multiline_dict=False the issue should also no occur.