Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it possible to relax the rule that put the code in one line if it can? #1054

Closed
llacroix opened this issue Oct 10, 2019 · 8 comments
Closed

Comments

@llacroix
Copy link

llacroix commented Oct 10, 2019

Howdy! Sorry you're having trouble. To expedite your experience,
provide some basics for me:

Operating system: Linux
Python version: 3.7
Black version:19.3b0

Here's a snippet of code:

    cost = fields.Float(
        compute="compute_cost",
        string="Cost"
    )

    report = fields.Text(
        compute="compute_cost",
        string="Report"
    )

That becomes:

    cost = fields.Float(compute="compute_cost", string="Cost")

    report = fields.Text(compute="compute_cost", string="Report")

Ideally, I would prefer if black didn't try to reduce the amount of lines and kept parameters on the same indent. The reason to do so is that the parameters on each line are at the same indent and it's easy to get from one to the other in vim without having to move the cursor only vertically. It also reduce eye movement so if you have a screen with high vertical resolution, it's easier to read code than moving eyes from left to right to read line of code.

In some case, I'd rather have it increase the amount of lines instead of packing the most it can on one line.

One idea thought is to reduce the amount of token on one line. So if we parse

cost = fields.Float(compute="compute_cost", string="Cost")

as

["cost", "=", "fields", ".", "Float", "(", "compute", "=", '"compute_cost"', ",", "string", "=", '"Cost"', ")"]

And the configuration is max 10 token on one line:
It would result with something like this:

cost = fields.Float(compute="compute",
string="Cost")

which would later get reformatted to:

cost = fields.Float(
    compute="compute", string="Cost"
)

Still not exactly what I'd like but having a setting that favor multiline over one line would be nice.

@zsol
Copy link
Collaborator

zsol commented Oct 13, 2019

What if Black would let you control how multiline expressions are exploded by checking for a trailing comma? #826 is working towards that direction, but the implementation unfortunately won't work for your use case (since these are neither collection literals nor top level expressions.

@llacroix
Copy link
Author

Yes, I think that could work and as it's likely to get merged soon.

but the implementation unfortunately won't work for your use case (since these are neither collection literals nor top level expressions.

I'll give a try to the branch, the example I posted are probably a little incomplete, they are nested inside a class so I believe this should be fine if we add a trailing comma. That would require some manual modifications to the code in some case but if I if works and all the team member start using it and use that convention, that would certainly remove a lot of manual work in our case.

@llacroix
Copy link
Author

I tried it but it doesn't work. It doesn't seem to reformat method call but only syntax for tuple, dict, list.

@thnee
Copy link

thnee commented Nov 4, 2019

I would really like to see this addition as well. I actually thought that #826 was going to allow this, but apparently it still forces everything to one line.

All I would like to see is that black allows me to choose between the two options. If a line is too long, black should still provide a default. But if I have decided I want to break a call into multiple lines, there is no reason for black to force some lines into the other style.

  • This change would not change any existing code already formatted with black.
  • This change increases readability and diffability.
  • This change should be done consistently for all things, such as:
    • Lists.
    • Dicts.
    • Multiple imports in one statement.
    • Calling things with multiple arguments.
    • Declaring things with multiple arguments.
    • Declaring classes with multiple parents.

This is much easier to read and diff. (Desired format).

def something(
    argument1: Optional[str] = "foo",
    argument2: Optional[Union[str, int]] = "bar",
):
    pass


something(
    argument1="asdf",
    argument2="qwer",
)

This is harder to read and diff. (Current format).

def something(
    argument1: Optional[str] = "foo", argument2: Optional[Union[str, int]] = "bar",
):
    pass


something(
    argument1="asdf", argument2="qwer",
)

@llacroix
Copy link
Author

llacroix commented Nov 4, 2019

@thnee exactly.

I did a fork that technically implement the leading command on function call but it doesn't seem to be very stable. I didn't look further into it yet.

https://github.com/llacroix/black/commit/ddc68aa1829c3bcabcad492975ae7ad70d5d7201

When calling black on black with this commit, it fails as it seems to give different results between passes. So if someone can fix my mistakes and make it work, I'd be happy. Or give some pointers on why it's not working.

@RickyDepop
Copy link

I, too, would love to be able to tell Black not to force everything to one line.

@mathewhodson
Copy link

I think this is fixed by pull request #1612.

@JelleZijlstra
Copy link
Collaborator

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants