1559193 - [Automated review][objective-c] clang-format wants to make lines longer than 80 chars

Status: RESOLVED INVALID

(Developer Infrastructure :: Source Code Analysis, defect, P2)

Description

[Jonathan Kew [:jfkthame]]

The clang-format patch proposed at https://phabricator.services.mozilla.com/D33796#1001108 wants to re-wrap lines such that they'll have a length that considerably exceeds 80 chars -- the longest one ends up around 100 chars.

Do we intentionally use an entirely different max line length in .mm files, or what's the basis for this?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:sylvestre, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Interesting finding. thanks!

The Google coding style for objective C says 100 chars:
http://google.github.io/styleguide/objcguide.html#line-length

which is what is implemented in clang-format:
https://github.com/llvm-mirror/clang/blob/master/lib/Format/Format.cpp#L885

We can force change that if we want.

Ehsan, what do you think?

Comment 3

[(no longer active)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #2)

Interesting finding. thanks!

The Google coding style for objective C says 100 chars:
http://google.github.io/styleguide/objcguide.html#line-length

Yep.

which is what is implemented in clang-format:
https://github.com/llvm-mirror/clang/blob/master/lib/Format/Format.cpp#L885

We can force change that if we want.

Ehsan, what do you think?

My inclination is to follow the default Google style here unless if there is a great reason to change things (CCing Markus for visibility so that he can voice his opinion.)

FWIW since the switch to formatting with clang-format, a number of people have approached me asking if we're open to increasing our column limit to 100 lines globally now that we can reformat the tree automatically. I've been meaning to delve into some of the past objections to this to see what the arguments against doing so were at the time but have yet to find the time to do so... Not that this discussion should block what we do here, just wanted to point out that there is a desire on behalf of some folks to increase to 100 character column limit globally, FWIW.

Comment 4

[Markus Stange [:mstange]]

I'm very happy about the fact that .mm files use 100 columns rather than 80 columns. Objective C conventions encourage rather verbose method names.
(Independently, I'm one of the people who would prefer a 100 column limit globally.)

Comment 5

[Sylvestre Ledru [:Sylvestre]]

ok, thanks.
I updated the doc: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style$compare?locale=en-US&to=1562111&from=1561421
So, this bug can be marked as invalid.

I would be also in favor of moving to 100 for C/C++. I can probably help with that project Ehsan.

Comment 6

[(no longer active)]

Great! I'll try to get in touch soon. :-)