Clang-format formatting change

217 views
Skip to first unread message

Rafal

unread,
Nov 28, 2014, 11:26:01 AM11/28/14
to chromi...@chromium.org
It seems like current version of clang-format formats arguments differently than some previous versions did. Previously it tried to align arguments either all on one line (if all fit) or each on separate line. Now it tries to fit as many arguments on the line as possible and only then adding a line break.

This is the output from the current version (3.6.0):

$ clang-format -style=Chromium temp.cc
void Foo() {
  object->MethodCall(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbb,
                     cccccccccccccccc);
}

Previous formatting would look like this:

void Foo() {
  object->MethodCall(
      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbb, cccccccccccccccc);
}

Is that an intentional change? Previous formatting was consistently followed for a long time so changing this now is a bit late.

Ryan Sleevi

unread,
Nov 28, 2014, 11:49:30 AM11/28/14
to rchlo...@opera.com, Chromium-dev
Yes.

clang-format will continue to evolve over time.

The new format matches the vast majority of the (pre-clang-format) Chromium idiom, is accepted by the style guide, and generally produces more readable code (semi-subjectively, but it allows more code on the screen rather than whitespace) 

Peter Kasting

unread,
Nov 29, 2014, 3:22:55 PM11/29/14
to Ryan Sleevi, rchlo...@opera.com, Chromium-dev
On Fri, Nov 28, 2014 at 8:49 AM, Ryan Sleevi <rsl...@chromium.org> wrote:
clang-format will continue to evolve over time.

This is really the critical takeaway.  clang-format formatting, like the Google and Chromium style guides themselves, has changed in the past and likely will change in the future.  It is not guaranteed to choose a style and "stick with it" forever.  Using clang-format extensively is not intended primarily to prevent style-related code churn, though changes in formatting hopefully aren't simply capricious.

On a bug where this change was debated some, I wrote:

"[Inconsistency in formatting over time] is really an argument for not mass-reformatting directories with clang-format to begin with; simply let clang-format be a tool people can run on changes as they happen, and declare that, within the modules you own, if a change has been clang-formatted, reviewers won't argue about style.  This eliminates the initial code churn, reduces the chances for churn over time, and still reaps the "don't argue" benefits that people have cited for using clang-format to begin with.

"Of course, if you already clang-formatted your whole module, you can't really go back -- but in that case, you can still avoid doing another mass-reformat, and simply switch to the more opportunistic rule I suggest."

Finally, one argument in favor of this change that Ryan didn't mention: clang-format's style for Google code has long been the "bin-packing" style it changed to in this case, but there was previously a Chromium-specific exception causing the preference for one-arg-per-line.  The Chromium style guide and historical precedent didn't seem to provide strong reasons why Chromium style should have ever differed here, so removing the exception simply unified clang-format's behavior with existing Google practice.

PK

Scott Graham

unread,
Dec 1, 2014, 1:16:47 PM12/1/14
to Peter Kasting, Ryan Sleevi, rchlo...@opera.com, Chromium-dev

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Rafal Chlodnicki

unread,
Dec 1, 2014, 3:03:20 PM12/1/14
to Peter Kasting, Scott Graham, Ryan Sleevi, Chromium-dev
I see it was discussed to death already so my opinion won't change much
but I agree with danakj that this change makes the code less readable.
I think chromium could live perfectly well without changing this.
Especially as the old way way of formatting is prevalent (by a huge
margin) in existing code so if nothing else, it would be good to keep it
this way for consistency.

On Mon, 01 Dec 2014 19:16:17 +0100, Scott Graham <sco...@chromium.org>
wrote:
--
Rafal

Peter Kasting

unread,
Dec 1, 2014, 5:05:09 PM12/1/14
to Rafal Chlodnicki, Scott Graham, Ryan Sleevi, Chromium-dev
On Mon, Dec 1, 2014 at 12:02 PM, Rafal Chlodnicki <rchlo...@opera.com> wrote:
Especially as the old way way of formatting is prevalent (by a huge margin) in existing code so if nothing else, it would be good to keep it this way for consistency.

Since you're referencing the discussion on that bug, you presumably read why "consistency" is not an argument that necessarily favors one of the two behaviors here, which includes notes on how "prevalent (by a huge margin)" is dependent on where in the codebase you're working, and isn't universally true (or false).

PK
Reply all
Reply to author
Forward
0 new messages