[webkit-dev] python coding style, PEP-8, and 80-column line widths

[Eric Seidel]

<bike-shedding>

I think 80 columns is a waste of time and hurts readability.

Instead of being smart about when we wrap code, 80 adheres to a
blanket rule, discourages long variable/function names, and needlessly
expands code vertically ignoring modern wider-than-long monitors.

The optparse code which was recently re-wrapped is one example of 80c
fail.  Impossible to tell in the wrapped version which arguments are
actually being listed (first argument to the function) because your
eye can't parse the start of each line.

That said.  I'd rather have you and Adam and Chris all working on the
Python than have folks lose interest due to code wrapping.  If the
consensus is 80c, I'll learn to deal. :)

The choice is either:
A. no wrapping rule to match the rest of WebKit
B. 80c to match the official PEP8 spec (and Google code).

I've chosen A. in the past.  Mostly to match my own personal bias I'm sure.

-eric

</bike-shedding>

On Wed, Apr 14, 2010 at 5:02 PM, Dirk Pranke <dpranke at chromium.org> wrote:
> Hi all,
>
> As I'm sure the discussions in the webkit meeting over the past two
> days made clear, it looks like most of our non-C++ code is getting
> written in Python. Back in January, Adam Barth posted this thread [1]
> where I thought it was agreed we would attempt to follow PEP 8 ([2] -
> the standard Python style guide) faithfully.
>
> The one (somewhat) controversial aspect of that style guide is that it
> requires a 79-column wide line length.
>
> More recently, as part of implementing a style checker for Python code
> [3], some have advocated that we should not follow that particular
> restriction, because (a) the WebKit C++ style guide does not follow
> that restriction, and (b) there is a reasonable amount of Python code
> that does not follow the rule today and would have to be brought into
> compliance. In particular, it was asked that we disable the long-line
> check until at least the existing code was brought into compliance.
>
> As far as (b) goes, there are currently (as of r57510) 1533 long lines
> out of the 35,616 lines found under WebKitTools/Scripts/webkitpy,
> occurring in 120 out 193 .py files. Most of the files that have any
> long lines have less than five [4].
>
> As far as disabling the check goes, I rather firmly believe that if we
> wish to follow an ~80 column line-length limitation, the check needs
> to be on by default, or else those who aren't used to this convention
> will not follow it and the problem will get worse, not better (as
> evidenced by [5]).
>
> So, do we want to follow an 80-column limit for Python code, or not,
> and if we do, should we enforce the rule by default or not?
>
> If the concern is the existing code base I also (reluctantly)
> volunteer to fix the long lines, if that influences the decisions.
> Although I personally believe that code that is less than 80 columns
> wide is easier to read and easier to maintain for a variety of
> reasons, I mostly volunteer to do this because it's just not many
> lines of code and I believe that having style guidelines that aren't
> followed are a Very Bad Thing and hence the fact that we have existing
> violations is a bad reason in this case to argue for not following the
> rule.
>
> Note that I do not particularly wish to argue the merits of this
> particular style choice, just make (and document) a decision. It's a
> religious position and the interweb is littered with existing
> arguments pro and con that we would likely just rehash here. I raise
> the issue at all only because I had thought that we made a decision
> and then it was changed in relative obscurity in a bug, rather than
> after being discussed here, whch struck me as not the right way to do
> this sort of thing.
>
> -- Dirk
>
> [1] https://lists.webkit.org/pipermail/webkit-dev/2010-January/011423.html
> [2] http://www.python.org/dev/peps/pep-0008/
> [3] https://bugs.webkit.org/show_bug.cgi?id=33639
> [4] Stats:
> % find WebKitTools/Scripts/webkitpy -name thirdparty -prune -o -name
> \*.py | wc -l  # 193 .py files
> % find WebKitTools/Scripts/webkitpy -name thirdparty -prune -o -name
> \*.py | xargs wc -l  # 35,316 lines of code
> % find WebKitTools/Scripts/webkitpy -name thirdparty -prune -o -name
> \*.py | xargs awk '{ if (length($0) > 79) printf("%s:%d %s\n",
> FILENAME, FNR, $0);}' # all of the long lines
> % find WebKitTools/Scripts/webkitpy -name thirdparty -prune -o -name
> \*.py | xargs awk '{ if (length($0) > 79) printf("%s\n", FILENAME);}'
> | sort | uniq -c | wc -l # 120 files w/ long lines
> % find WebKitTools/Scripts/webkitpy -name thirdparty -prune -o -name
> \*.py | xargs awk '{ if (length($0) > 79) printf("%s\n", FILENAME);}'
> | sort | uniq -c # distribution by file
> [5] https://bugs.webkit.org/show_bug.cgi?id=37586
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>