autopep8 icon indicating copy to clipboard operation
autopep8 copied to clipboard

E128 causes E501 problems

Open jreinstra opened this issue 9 years ago • 3 comments

When I ran autopep8 on a file that had an instance of E128 (visual indent issue), autopep8 fixed the visual indent issue but caused the line length to be too long, creating an instance of E501 (line length - 82 chars instead of 79).

This shouldn't occur - if there's a conflict and both changes cannot be fixed, E501 (line length) should take precedence over smaller visual issues like E128.

Example of this happening: (after visual indent)

query_this_month = query.phorms(start, end,
                                qoeivtjdidje[spid_start:spid_end])
bait_last_years = (bringee_and_deploy(query_last_month,
                                      db=testtest.NEWERSS_DB)).kkoikfme()

jreinstra avatar Jul 20 '16 16:07 jreinstra

For my own usage, I think that alignment is more important than fitting within 79 characters. I find badly aligned code hard to read, while code with a few characters over 79 is not. But I welcome a pull request to add an option to do otherwise.

By the way, the example you provided is well within the 79-character limit.

myint avatar Jul 23 '16 15:07 myint

Yeah, I see your point. However, I (and I suspect others) likely prefer proper line length over proper alignment. It would be nice to be able to either specify which issues to prioritize when running into cases where one of two issues is inevitable or to find a way for autopep8 to resolve line length issues while keeping proper alignment.

jreinstra avatar Jul 25 '16 06:07 jreinstra

I think that alignment is more important than fitting

others likely prefer proper line length over proper alignment.

where one of two issues is inevitable

I find this conflict all the time, but I don't think that one of two issues is inevitable. In every case I've encountered, even in heavily nested dictionaries, it's possible to reindent to satisfy both.

In the example given, based on other output I've seen from autopep8, I'd expect to see this in the output:

        query_this_month = query.phorms(
            start, end, qoeivtjdidje[spid_start:spid_end])
        bait_last_years = (
            bringee_and_deploy(
                query_last_month,
                db=testtest.NEWERSS_DB)).kkoikfme()

It satisfies both E128 and E501, presumably satisfies the intention of the authors of PEP 8, and meets the goals of autopep8 (whitespace-only changes).

I'd also be satisfied with other complaint validations.

I think it's worthwhile postponing any 'conflict prioritization' and focusing first on addressing these slightly harder yet not inevitable conflicts.

jaraco avatar Nov 06 '17 20:11 jaraco