[PATCH] D22505: Access Modifier Use Normal Indent
LokiAstari created this revision.
LokiAstari added reviewers: djasper, klimek.
LokiAstari added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
Access Modifiers (public/protected/private) causes standard indent.
```
class MyClass
{
int value1;// standard indent.
public:// access modifier (increases indent level)
int value2;// next item indent one more level
private: // access modifier goes out a level
int value3;// next item indent one more level
};
```
https://reviews.llvm.org/D22505
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.h
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/CMakeLists.txt
unittests/Format/FormatTestAccess.cpp
Index: unittests/Format/FormatTestAccess.cpp
===
--- unittests/Format/FormatTestAccess.cpp
+++ unittests/Format/FormatTestAccess.cpp
@@ -0,0 +1,150 @@
+//===- unittest/Format/FormatTestAccess.cpp - Formatting unit tests
+//-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "../Tooling/RewriterTestContext.h"
+#include "FormatTestUtils.h"
+
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class FormatTestAccess : public ::testing::Test {
+protected:
+ std::string format(llvm::StringRef Code, bool normalFormat) {
+const FormatStyle &Style = getGoogleStyleWithColumns(normalFormat);
+DEBUG(llvm::errs() << "---\n");
+DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(0, Code.size()));
+bool IncompleteFormat = false;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", &IncompleteFormat);
+EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+ }
+
+ FormatStyle getGoogleStyleWithColumns(bool normalFormat) {
+FormatStyle Style = getLLVMStyle();
+Style.AccessModifierOffset = normalFormat ? 0 : -2;
+Style.AccessModifierStandardIndent = normalFormat;
+return Style;
+ }
+
+ void verifyFormat(llvm::StringRef Code, bool normalFormat) {
+EXPECT_EQ(Code.str(), format(test::messUp(Code), normalFormat));
+ }
+};
+
+TEST_F(FormatTestAccess, NoChangeWithoutAccess) {
+ verifyFormat("class A {\n"
+ " int a1;\n"
+ "};",
+ false);
+ verifyFormat("class B {\n"
+ " int b1;\n"
+ "};",
+ true);
+}
+
+TEST_F(FormatTestAccess, ChangeWithAccess) {
+ verifyFormat("class C {\n"
+ " int c1;\n"
+ "public:\n"
+ " int c2;\n"
+ "};",
+ false);
+ verifyFormat("class D {\n"
+ " int d1;\n"
+ " public:\n"
+ "int d2;\n"
+ "};",
+ true);
+}
+
+TEST_F(FormatTestAccess, MultipleAccessLevels) {
+ verifyFormat("class E {\n"
+ " int e1;\n"
+ "public:\n"
+ " int e2;\n"
+ "private:\n"
+ " int e3;\n"
+ "protected:\n"
+ " int e4;\n"
+ "};",
+ false);
+ verifyFormat("class F {\n"
+ " int f1;\n"
+ " public:\n"
+ "int f2;\n"
+ " private:\n"
+ "int f3;\n"
+ " protected:\n"
+ "int f4;\n"
+ "};",
+ true);
+}
+
+TEST_F(FormatTestAccess, NestedClass) {
+ verifyFormat("class G {\n"
+ " int g1;\n"
+ " class GGA {\n"
+ "int gg1;\n"
+ " public:\n"
+ "int gg2;\n"
+ " };\n"
+ "public:\n"
+ " class GGB {\n"
+ "int gg1;\n"
+ " public:\n"
+ "int gg2;\n"
+ " };\n"
+ " int g2;\n"
+ "private:\n"
+ " int g3;\n"
+ "protected:\n"
+ " int g4;\n"
+ "};",
+ false);
+ verifyFormat("class H {\n"
+ " int h1;\n"
+ " class HHA {\n"
+ "int hh1;\n"
+ "public:\n"
+
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. Each new style option must :: - be used in a project of significant size (have dozens of contributors) - have a publicly accessible style guide - have a person willing to contribute and maintain patches Example: https://github.com/openframeworks/openFrameworks Style Guide: https://github.com/openframeworks/openFrameworks/wiki/oF-code-style#indentation Contributors: 220 Willing to maintain: Me (Loki Astari. I have more patches I want to contribute so I am not doing a drive bye :-) https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. @djasper@klimek Is that a sufficient example? Or do I need to do some more exhaustive search of github for projects? https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. @djasper@klimek Hi guys, I am not sure how to move this PR forward. I am not sure if you are uninterested or if this is two minor in comparison to other issues. If there is some other task I need to do to move this forward I more than happy to do some leg work, but given the documentation I am not sure what else I need to do. Loki. https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. Hope you don't mind me chiming in: > Roughly go ahead with what you are suggesting, although the option should not > be called AccessModifierStandardIndent, as that carries no meaning and > actually is far from the "standard" way. Reasonable names that spring to mind > are: Though I personally disagree on the more standard technique :-) Since I am the newbie to this I am not going to fight on that point (though I suspect if we meet up for some beers we could have a spirited debate). > AccessModifiersCauseAdditionalIndent or > (Additional)IndentAfterAccessModifiers. How about "AccessModifiersUseNormaIndent" as we are indenting by a normal indent width. But I am not going to fight over the name when it comes down to it. > Be even more "clever" with AccessModifierOffset (I can come up with various > ways from negative numbers to special numbers, none of them really good). I don't think this is a good idea (but as the newbie no strong feelings). Unfortunately a lot of large projects have stated using this is a standard part of their formatting; got to this project too late to prevent that :-O > Deprecate AccessModifierOffset and instead use an enum with a few dedicated > settings. The issue with this is getting all projects to update their settings with the new version. Not sure this is a great user experience. https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. @djasper@klimek Just want to bring this to the top of your queue again :-) Any further thoughts on the processes? Loki https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment.
I don't have a problem changing it so the default behaviour is:
class C {
int v1;
private:
int v2;
};
But I would like to retain the functionality that if there is no explicit
public/private/protected that it follows the original.
But if there is huge objection to that I am not going to fight over this one (I
can live with the indention above).
Loki.
https://reviews.llvm.org/D22505
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D22505: clang-format Access Modifier Use Normal Indent
LokiAstari added a comment. > That should already be doable with a negative offset today, right? Yes. So I don't need to add any changes. right? https://reviews.llvm.org/D22505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
