Yapf: Add knob to never ever break line after "'dictkey':"

[rafalp]

Currently if one or more of dict's entries are longer than column_limit (or simply contain multiline string), yapf will break line after all dict's keys in dict.

I've tried disabling indent_dictionary_value but this makes yapf just align value right under key, which is something I dislike more than breaking line and inlining value ;)

Eventually, I've developed custom utility for my project that performs my way cleanup after yapf, but after 12h of programmering I can't help but feel that its hacky and fragile, which is why I'm opening ticket here.

[gwelymernans]

Do you want it to never split, even if it goes over the column limit? For example, this:

X = {
    'key': this_is_a_function_call_that_goes_over_the_column_limit_im_pretty_sure()
}

[Daenyth]

Yes, I'd want that to not split, or if it does, it should be the only key that splits. The problem is right now you go from this:

X = {
    'a': 1,
    'b': 2,
    'key': this_is_a_function_call_that_goes_over_the_column_limit_im_pretty_sure()
}

to this:

X = {
    'a': 
        1,
    'b': 
        2,
    'key': 
        this_is_a_function_call_that_goes_over_the_column_limit_im_pretty_sure()
}

Where I'd prefer either not splitting that line at all, or

X = {
    'a': 1,
    'b': 2,
    'key': 
        this_is_a_function_call_that_goes_over_the_column_limit_im_pretty_sure()
}

[rafalp]

@gwelymernans exactly. Let it be, because in practice this really hurts translation strings in django projects:

some_dict = {
    'title': _("I am example data"),
    'description': _(
        "Lorem ipsum dolor met sit amet elit, si vis pacem para bellum "
        "elites nihi very long string."
    ),
}

I have a lot of bits like this, and they all result in this:

some_dict = {
    'title':
        _("I am example data"),
    'description':
        _(
            "Lorem ipsum dolor met sit amet elit, si vis pacem para bellum "
            "elites nihi very long string."
        ),
}

[gwelymernans]

For translation strings, you can specify I18N_FUNCTION_CALL=['_'], and it shouldn't modify things. (Similarly for i18n comments.)

I'll see what I can do about adding a knob for this.

[gwelymernans]

Could you test out the patch for me? It's in this branch:

https://github.com/google/yapf/tree/no-split-before-dict-value

[rafalp]

It's partial success. I does what @Daenyth asked for, but not what I'm after.

This code:

attrs = {
    'category': category,
    'role': forms.ModelChoiceField(
        label=_("Role"),
        required=False,
        queryset=category_roles,
        initial=selected_role,
        empty_label=_("No access"),
    ),
}

Is turned into this:

attrs = {
    'category': category,
    'role':
        forms.ModelChoiceField(
            label=_("Role"),
            required=False,
            queryset=category_roles,
            initial=selected_role,
            empty_label=_("No access"),
        ),
}

Whereas in scenario I'm after it would be left unchanged.

On sidenote I've spotted places in yapf where it meddles with named assigns in similiar fasion:

css_class = forms.CharField(
    label=_("CSS class"),
    required=False,
    help_text=_(
        "Optional CSS class used to customize this category appearance from templates."
    ),
)

Turns into:

css_class = forms.CharField(
    label=_("CSS class"),
    required=False,
    help_text=
    _("Optional CSS class used to customize this category appearance from templates."),
)

My yapf:

[style]
coalesce_brackets = true
column_limit=99
dedent_closing_brackets = true
each_dict_entry_on_separate_line = true
i18n_function_call= _
indent_dictionary_value = true
join_multiple_lines = false
no_split_before_dict_value = true
split_arguments_when_comma_terminated = true
split_before_first_argument = true
split_before_logical_operator = true
split_before_named_assigns = true

[Daenyth]

Mine. Pretty sure much of this is generated from defaults

[style]
based_on_style = pep8
ALIGN_CLOSING_BRACKET_WITH_VISUAL_INDENT=True
ALLOW_MULTILINE_LAMBDAS=True
BLANK_LINE_BEFORE_NESTED_CLASS_OR_DEF=False
COALESCE_BRACKETS=False
COLUMN_LIMIT=120
CONTINUATION_INDENT_WIDTH=4
DEDENT_CLOSING_BRACKETS=True
EACH_DICT_ENTRY_ON_SEPARATE_LINE=True
I18N_COMMENT=
I18N_FUNCTION_CALL=
INDENT_DICTIONARY_VALUE=True
INDENT_WIDTH=4
JOIN_MULTIPLE_LINES=True
SPACES_AROUND_DEFAULT_OR_NAMED_ASSIGN=False
SPACES_AROUND_POWER_OPERATOR=False
SPACES_BEFORE_COMMENT=2
SPACE_BETWEEN_ENDING_COMMA_AND_CLOSING_BRACKET=True
SPLIT_ARGUMENTS_WHEN_COMMA_TERMINATED=False
SPLIT_BEFORE_BITWISE_OPERATOR=False
SPLIT_BEFORE_DICT_SET_GENERATOR=True
SPLIT_BEFORE_FIRST_ARGUMENT=True
SPLIT_BEFORE_LOGICAL_OPERATOR=True
SPLIT_BEFORE_NAMED_ASSIGNS=True
SPLIT_PENALTY_AFTER_OPENING_BRACKET=30
SPLIT_PENALTY_AFTER_UNARY_OPERATOR=10000
SPLIT_PENALTY_BEFORE_IF_EXPR=0
SPLIT_PENALTY_BITWISE_OPERATOR=300
SPLIT_PENALTY_EXCESS_CHARACTER=2600
SPLIT_PENALTY_FOR_ADDED_LINE_SPLIT=30
SPLIT_PENALTY_IMPORT_NAMES=0
SPLIT_PENALTY_LOGICAL_OPERATOR=300
USE_TABS=False

[felixc]

Based on the description in this comment, it sounds like the patch goes at least some way to solving the issue. Personally I'd be happy with the behaviour described there!

Is it worth merging those changes as they are, even if folks want to do more work in this area in the future?

[Daenyth]

I'd really like it

[audiolion]

it seems like the OP asked for never split or only split the over limit key, @gwelymernans gave the second option, and also mentioned the i18n settings to ignore translation strings which when combined with this patch should solve OP's issue.

Later if there is enough need an incremental add for never splitting the line of the key could be added and just splitting after function arguments

[rafalp]

Now hold on here, @audiolion. I can confirm that I've never got where I've hoped to with pure Yapf with this issue. I know my original example contained translation string, but that was simply offhand example (that btw I've failed to spot improvement with via i18n settings).

My wish would be to don't break too long lines for dict values as well as function/object kwargs. That way those functions tip off the pylint and let me properly fix my code.

[mikelambert]

@gwelymernans , any plans to integrate https://github.com/google/yapf/commit/848efddd27a8547ecce0876eeed12c197a3d4234 -et-al? Or should I not expect that to reach master?

[petobens]

@gwelymernans I would also like https://github.com/google/yapf/commit/848efddd27a8547ecce0876eeed12c197a3d4234 to be merged :)

[gwelymernans]

Sorry about the late response, guys. This fell off my plate. I'm going to try for a more cohesive approach that @rafalp suggested. If that falls through, I'll go for 848efdd.

[humitos]

Thanks! I just tried it from master and it works as I expected! Thanks a lot!

[evanyeatts]

Is there an equivalent knob for optional parameters in a function/constructor call? Perhaps this knob could be updated to affect such a case as well?

Example of current behavior:

SchemaField(
    'name',
    'STRING',
    mode='required',
    description=
    'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc fringilla faucibus augue sed luctus. Nullam sapien leo, faucibus ac metus id, lobortis consectetur turpis. Phasellus imperdiet nisl pretium quam bibendum condimentum. Nulla sit amet mauris molestie, congue ipsum sed, mattis ex. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer dignissim faucibus ipsum, id iaculis dolor mattis sed. Mauris egestas aliquam dui, eget lobortis erat fermentum aliquet. Morbi iaculis augue rhoncus nisl sodales pharetra. Vivamus malesuada, lectus vel porta vestibulum, diam sem viverra nulla, eget consectetur nulla felis eget libero. Etiam sed velit vitae sem eleifend blandit. Donec viverra libero lorem, eu luctus tortor faucibus fermentum. Mauris vehicula ornare metus pharetra eleifend.',
),

[gwelymernans]

@evanyeatts Could you open a separate issue for that?

[evanyeatts]

@gwelymernans Done: https://github.com/google/yapf/issues/480