This is an archive of the discontinued LLVM Phabricator instance.

Update Mozilla Style
Needs ReviewPublic

Authored by kentuckyfriedtakahe on Jan 23 2014, 5:47 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Update Mozilla style to more closely match the official Mozilla guidelines

Diff Detail

Event Timeline

Discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=961541 supports leaving IndentCaseLabels as is but having ContinuationIdentWidth set to 2.

kentuckyfriedtakahe updated this revision to Unknown Object (????).Jan 23 2014, 5:58 PM

This allows breaking after the type in top level function definitions.

int
MyClass::MyFunction();

I tried a few different ways to implement this feature but this is what it took to make it work. I tried implementing it in ContinuationIndenter::mustBreak but State.Line->MightBeFunctionDecl seems to be false for functions with no arguments.

djasper added inline comments.Jan 23 2014, 9:38 PM
include/clang/Format/Format.h
252

We are trying to move towards fewer and more generic options. So my initial question would be, are you sure that this applies only for top-level functions? I couldn't find anything in the style guide.. Also, I didn't find anything in the GNU style guide. The same is true for the question: Does this apply to both declarations and definitions? So, I think we should at least make it easy to enable this also for non-top-level functions.

So, more specifically, I would make this an enum:

enum BreakReturnType { // Name might not be ideal.
  BRT_Always,
  BRT_TopLevel,
  BRT_Default,
  BRT_LastResort
};

And this can then also replace the current PenaltyReturnTypeOnItsOwnLine (using 60 for BRT_TopLevel, and BRT_Default and 200 for BRT_LastResort).

Then I would probably call the style option:

BreakReturnType BreakFunctionsAfterType;
lib/Format/Format.cpp
339

Shouldn't this contain your other modifications?

342

Whatever this parameter ends up being called, please also set it for GNU style (llvm.org/PR 17851).

lib/Format/TokenAnnotator.cpp
1087

The name 'FirstFunctionDecl' doesn't make much sense to me as there aren't multiple function declarations in one line. I guess you'd want to exclude trailing annotations? Can you add tests testing this and the other branches (top-level vs. not-top-level) in unittests/Format/FormatTests.cpp?

1105

No line break after "}".. (Use clang-format :-) )

Ping? Any update on this?

Daniel,

Thanks for reminding me. I've just brought up clang-format for further
bike shedding on dev-platform to see what people think. I'll get back to
you next week about it.

Anthony

Any updates?

djasper resigned from this revision.Apr 12 2015, 1:22 AM
djasper removed a reviewer: djasper.