Page MenuHomePhabricator

[clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
AbandonedPublic

Authored by MyDeveloperDay on Apr 3 2019, 12:24 PM.

Details

Summary

Code::Blocks and Qt Creator code uses a style guide which recommends indenting the next line after the access modifier e.g.

class Foo
{
          int abc;

      public:
          int def;
          void foo();

      private:
          int ghi;
          void foo();
}

The following patch with the addition of a new option

AccessModifierIndentation: true

This PR (https://bugs.llvm.org/show_bug.cgi?id=19056) was raised a long time ago (2014), and recently was pinged for an update

The following patch aim to address this issue, potentially allowing clang-format to be used (rather than astyle)

Diff Detail

Event Timeline

MyDeveloperDay created this revision.Apr 3 2019, 12:24 PM
MyDeveloperDay edited the summary of this revision. (Show Details)
MyDeveloperDay edited the summary of this revision. (Show Details)
3nids added a subscriber: 3nids.Apr 3 2019, 1:48 PM
MyDeveloperDay set the repository for this revision to rC Clang.Apr 9 2019, 12:32 AM
MyDeveloperDay edited projects, added Restricted Project; removed Restricted Project.
klimek added inline comments.Apr 9 2019, 1:08 AM
clang/include/clang/Format/Format.h
50

I think we need to explain this a bit more:
What this does is:
Indent classes with access modifiers at 2x indent compared to classes without access modifiers, while keeping the access modifiers at a normal indent.

clang/lib/Format/UnwrappedLineParser.cpp
2009

What if the class starts at level 1? (for example, inside a function or due to namespace indentation)

namespace A {

class B {
  public:
    ..
};

}

2022

Similarly, I think we need to remember whether we unindented, as otherwise I think we can run into cases where this is true, but the previous was false (class declared at level > 1).

2230

We should structure this like other parse loops in this file, using switch and eof (instead of !Next). See parseParens() for a good example.

reuk added inline comments.Apr 9 2019, 2:25 AM
clang/lib/Format/UnwrappedLineParser.cpp
2009

Probably worth adding a test or two that format nested classes, classes inside namespaces, classes defined inside functions etc.

2253

Yeah, this would look way nicer using a settings struct.

clang/lib/Format/UnwrappedLineParser.h
89

Adjacent bool parameters is a bit nasty. Maybe pass a struct of settings instead? https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i4-make-interfaces-precisely-and-strongly-typed

JonasToth resigned from this revision.Apr 9 2019, 10:49 AM
MyDeveloperDay abandoned this revision.Jun 20 2019, 11:18 AM

Hi, may I ask why this is abandoned?
We are eagerly waiting on this to move to clang-format.

It was more about not having time to pursue it at this time.. I didn't feel I should hog the PR if someone else wanted to take a look.

Even though its "Abandoned" it can be brought back at anytime, I don't like to sit on reviews for too long as it doesn't show the correct intent. I'm happy for others to take it over, but I don't have a personal need for this.