Bug 115629 – check-webkit-style should allow arbitrary number indentation spaces for multi-line expressions

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Balazs Kelemen]

I find it odd that I cannot indent for example the arguments of a call like this:
func(a, b, c
     d, e)
It complains that the second line is not indented with multiply of 4 spaces.

[Alexey Proskuryakov]

This is regular WebKit style: no fancy formatting that will get eventually broken with global search-and-replace, just four spaces.

[Balazs Kelemen]

Could you give example of files which are actually consistent with this rule? I think the majority of webkit does not keep it (which is good because it is a stupid rule).

[Darin Adler]

We often avoid this by not having function calls so long they have to break into multiple lines. That’s what I prefer in most cases.

This rule is relatively clear in the WebKit style guide, although it could be clearer, so it’s not an issue with the checking script but rather with the style guide. We’d want to change the style guide if had consensus that we want to change the style. There are examples of the style in the style guide itself, for example in http://www.webkit.org/coding/coding-style.html#braces-one-line there is a call to myFunction that follows the rule.

As Alexey said, the rationale for the rule is that when we change names of things, we don’t want to have to look at each replacement site and possibly reindent code.

As far as an example is concerned, I tried to think of a source file with long function call lines and went to FrameLoader.cpp and found these multi-line function calls: <http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp?rev=149589#L1212> <http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp?rev=149589#L1229> <http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp?rev=149589#L1370>. I don’t want to spend more time finding examples.

One reason at least some people like the style of lining things up with the parenthesis is because their editor does it automatically. I do think that new contributors often get this one wrong and just land what their editor does or what they are used to in other projects.

[Balazs Kelemen]

> As Alexey said, the rationale for the rule is that when we change names of things, we don’t want to have to look at each replacement site and possibly reindent code.

Hmm, yes that makes sense, I did not really think it over. However I still think it is bad for readability but it's just my sense of taste.

[Balazs Kelemen]

I realized I cannot even do this:
var = func(long, argument, list, ...
        and, some, more)

This is 4 space indent, but check-webkit-styles complains that I should indent the second line with only 1x4 spaces.
Don't you think this is extremely ugly?
var = func(a, b, c
    d, e, f)

I think life was better back when check-webkit-style was a simple script that did not try to cover every situation of coding.

[Ryosuke Niwa]

Consistency is important.