[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
3nids added a comment. Hi there, is there anything we can do to help this to go through? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60225/new/ https://reviews.llvm.org/D60225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
MyDeveloperDay added a comment. 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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60225/new/ https://reviews.llvm.org/D60225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
3nids added a comment. Hi, may I ask why this is abandoned? We are eagerly waiting on this to move to clang-format. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60225/new/ https://reviews.llvm.org/D60225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
reuk added inline comments.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+ // classes case
+ if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+--Line->Level;
klimek wrote:
> What if the class starts at level 1? (for example, inside a function or due
> to namespace indentation)
>
> namespace A {
> class B {
> public:
> ..
> };
> }
Probably worth adding a test or two that format nested classes, classes inside
namespaces, classes defined inside functions etc.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2253
parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
- /*MunchSemi=*/false);
+ /*MunchSemi=*/false,
+ /*IndentedAccessModifiers=*/ContainsAccessModifier);
Yeah, this would look way nicer using a settings struct.
Comment at: clang/lib/Format/UnwrappedLineParser.h:89
void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
- bool MunchSemi = true);
+ bool MunchSemi = true, bool IndentedAccessModifiers = false);
void parseChildBlock();
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
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60225/new/
https://reviews.llvm.org/D60225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
klimek added inline comments.
Comment at: clang/include/clang/Format/Format.h:50
struct FormatStyle {
+ /// Indents after access modifiers. i.e.
+ /// \code
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.
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+ // classes case
+ if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+--Line->Level;
What if the class starts at level 1? (for example, inside a function or due to
namespace indentation)
namespace A {
class B {
public:
..
};
}
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2022
+ // After we wrap for Access modifier then indent a level if desired
+ if (Style.AccessModifierIndentation && Line->Level >= 1)
+++Line->Level;
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).
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2230
+ Next = Tokens->getNextToken();
+ if (!Next)
+break;
We should structure this like other parse loops in this file, using switch and
eof (instead of !Next). See parseParens() for a good example.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60225/new/
https://reviews.llvm.org/D60225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, djasper, JonasToth, russellmcc, reuk.
MyDeveloperDay added a project: clang-tools-extra.
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 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)
https://reviews.llvm.org/D60225
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1982,6 +1982,209 @@
Style));
}
+TEST_F(FormatTest, FormatAccessModifierIndentation) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AccessModifierIndentation = false;
+ EXPECT_EQ("class Foo1 {\n"
+" int i;\n"
+"\n"
+"public:\n"
+" int j;\n"
+" int k;\n"
+" Foo1() {}\n"
+"\n"
+"private:\n"
+" Bar() {}\n"
+" int l;\n"
+" int m;\n"
+"\n"
+"protected:\n"
+" Ping() {}\n"
+" int n;\n"
+" int o;\n"
+"};\n",
+format("class Foo1 {\n"
+ "int i;\n"
+ "public:\n"
+ "int j;\n"
+ "int k;\n"
+ "Foo1() {}\n"
+ "private:\n"
+ "Bar() {}\n"
+ "int l;\n"
+ "int m;\n"
+ "protected:\n"
+ "Ping() {}\n"
+ "int n;\n"
+ "int o;\n"
+ "};\n",
+ Style));
+
+ Style.AccessModifierIndentation = true;
+ EXPECT_EQ("class Foo2 {\n"
+"int i;\n"
+"\n"
+"public:\n"
+"int j;\n"
+"int k;\n"
+"Foo2() {}\n"
+"\n"
+"private:\n"
+"Bar() {}\n"
+"int l;\n"
+"int m;\n"
+"\n"
+"protected:\n"
+"Ping() {}\n"
+"int n;\n"
+"int o;\n"
+"};\n",
+format("class Foo2 {\n"
+ "int i;\n"
+ "public:\n"
+ "int j;\n"
+ "int k;\n"
+ "Foo2() {}\n"
+ "private:\n"
+ "Bar() {}\n"
+ "int l;\n"
+ "int m;\n"
+ "protected:\n"
+ "Ping() {}\n"
+ "int n;\n"
+ "int o;\n"
+ "};\n",
+ Style));
+
+ Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+ Style.AccessModifierIndentation = false;
+ EXPECT_EQ("class Foo3\n"
+"{\n"
+" int i;\n"
+"\n"
+"public:\n"
+" int j;\n"
+" int k;\n"
+" Foo3() {}\n"
+"\n"
+"private:\n"
+" Bar() {}\n"
+" int l;\n"
+" int m;\n"
+"\n"
+"protected:\n"
+" Ping() {}\n"
+" int n;\n"
+" int o;\n"
+"};\n",
+format("class Foo3 {\n"
+ "int i;\n"
+ "public:\n"
+ "int j;\n"
+ "int k;\n"
+ "Foo3() {}\n"
+ "private:\n"
+ "Bar() {}\n"
+ "int l;\n"
+ "int m;\n"
+ "protected:\n"
+ "Ping() {}\n"
+ "int n;\n"
+ "int o;\n"
+ "};\n",
+ Style));
+
+ Style.AccessModifierIndentation = true;
+ EXPECT_EQ("class Foo4\n"
+"{\n"
+"int i;\n"
+"\n"
+"public:\n"
+"int j;\n"
+"int k;\n"
+"Foo4() {}\n"
+"\n"
+"private:\n"
+"Bar() {}\n"
+"int l;\n"
+"int m;\n"
+"\n"
+"protected:\n"
+"Ping() {}\n"
+"int n;\n"
+"int
