[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz updated this revision to Diff 191540.
to-miz added a comment.
Made a new diff on the git repository, since the docs now recommend using git.
Rebased and created a new diff with `git show HEAD -U99`.
The patch applies cleanly on windows after `git reset --hard` so I hope there
shouldn't be any merge conflicts now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D52150/new/
https://reviews.llvm.org/D52150
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2993,22 +2993,25 @@
EXPECT_EQ(Expected, format(ToFormat, Style));
EXPECT_EQ(Expected, format(Expected, Style));
}
- // Test with tabs.
- Style.UseTab = FormatStyle::UT_Always;
- Style.IndentWidth = 8;
- Style.TabWidth = 8;
- verifyFormat("#ifdef _WIN32\n"
- "#\tdefine A 0\n"
- "#\tifdef VAR2\n"
- "#\t\tdefine B 1\n"
- "#\t\tinclude \n"
- "#\t\tdefine MACRO \\\n"
- "\t\t\tsome_very_long_func_aa();\n"
- "#\tendif\n"
- "#else\n"
- "#\tdefine A 1\n"
- "#endif",
- Style);
+ // Test AfterHash with tabs.
+ {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+ }
// Regression test: Multiline-macro inside include guards.
verifyFormat("#ifndef HEADER_H\n"
@@ -3018,6 +3021,102 @@
" int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ // Basic before hash indent tests
+ verifyFormat("#ifdef _WIN32\n"
+ " #define A 0\n"
+ " #ifdef VAR2\n"
+ "#define B 1\n"
+ "#include \n"
+ "#define MACRO \\\n"
+ " some_very_long_func_aa();\n"
+ " #endif\n"
+ "#else\n"
+ " #define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ " #define MACRO\\\n"
+ "void a(int x) {\\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ "}\n"
+ "#endif",
+ Style);
+ // Keep comments aligned with indented directives. These
+ // tests cannot use verifyFormat because messUp manipulates leading
+ // whitespace.
+ {
+const char *Expected = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ " // Aligned to code.\n"
+ " int a;\n"
+ " #if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ " // Aligned to code.\n"
+ " int b;\n"
+ " #endif\n"
+ "#endif\n"
+ "}";
+const char *ToFormat = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ "// Aligned to code.\n"
+ "int a;\n"
+ "#if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ "// Aligned to code.\n"
+ "int b;\n"
+ "#endif\n"
+ "#endif\n"
+ "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+ }
+ {
+const char *Expected =
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz added a comment. I checked out the git repository and applied the patch to head and reran all tests. The diff produced by git is basically the same, should I upload the new diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz added a comment. Is it because the diff was made with the svn repository? It is for the newest revision. Should I make a new diff with the git repo instead? Or is it not because of the diff, but something in phabricator instead? Sorry this is my first patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz added a comment. I don't have commit access. It would be nice if someone could commit it in my place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz updated this revision to Diff 189048.
to-miz added a comment.
Added an entry to ReleaseNotes.rst about the new option.
This diff doesn't contain unrelated formatting changes due to running
clang-format on some source files that apparently weren't formatted before.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D52150/new/
https://reviews.llvm.org/D52150
Files:
docs/ClangFormatStyleOptions.rst
docs/ReleaseNotes.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1812,8 +1812,8 @@
Style.CompactNamespaces = true;
verifyFormat("namespace A { namespace B {\n"
- "}} // namespace A::B",
- Style);
+ "}} // namespace A::B",
+ Style);
EXPECT_EQ("namespace out { namespace in {\n"
"}} // namespace out::in",
@@ -2953,22 +2953,25 @@
EXPECT_EQ(Expected, format(ToFormat, Style));
EXPECT_EQ(Expected, format(Expected, Style));
}
- // Test with tabs.
- Style.UseTab = FormatStyle::UT_Always;
- Style.IndentWidth = 8;
- Style.TabWidth = 8;
- verifyFormat("#ifdef _WIN32\n"
- "#\tdefine A 0\n"
- "#\tifdef VAR2\n"
- "#\t\tdefine B 1\n"
- "#\t\tinclude \n"
- "#\t\tdefine MACRO \\\n"
- "\t\t\tsome_very_long_func_aa();\n"
- "#\tendif\n"
- "#else\n"
- "#\tdefine A 1\n"
- "#endif",
- Style);
+ // Test AfterHash with tabs.
+ {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+ }
// Regression test: Multiline-macro inside include guards.
verifyFormat("#ifndef HEADER_H\n"
@@ -2978,6 +2981,102 @@
" int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ // Basic before hash indent tests
+ verifyFormat("#ifdef _WIN32\n"
+ " #define A 0\n"
+ " #ifdef VAR2\n"
+ "#define B 1\n"
+ "#include \n"
+ "#define MACRO \\\n"
+ " some_very_long_func_aa();\n"
+ " #endif\n"
+ "#else\n"
+ " #define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ " #define MACRO\\\n"
+ "void a(int x) {\\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ "}\n"
+ "#endif",
+ Style);
+ // Keep comments aligned with indented directives. These
+ // tests cannot use verifyFormat because messUp manipulates leading
+ // whitespace.
+ {
+const char *Expected = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ " // Aligned to code.\n"
+ " int a;\n"
+ " #if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ " // Aligned to code.\n"
+ " int b;\n"
+ " #endif\n"
+ "#endif\n"
+ "}";
+const char *ToFormat = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ "// Aligned to code.\n"
+ "int a;\n"
+ "#if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ "// Aligned to code.\n"
+ "int b;\n"
+ "#endif\n"
+
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz added a comment. I understand that adding new options adds costs to the development of clang-format, but there was a feature request and there is interest for this option. As for the cost to the development, there already exists PPDIS_AfterHash. All I did was make the changes introduced in that patch a bit more general and removed the hard coded assumption that there are no indents before a hash at a couple of places. I would argue the cost of development of this new option is roughly the same as it is with the existence of PPDIS_AfterHash itself. PPDIS_BeforeHash seems like a natural extension of the changes introduced by PPDIS_AfterHash. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz updated this revision to Diff 165700.
to-miz added a comment.
PPBranchLevel can be negative, while Line->Level is unsigned. Added check to
ensure that PPBranchLevel is positive before adding to Line->Level.
https://reviews.llvm.org/D52150
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1754,8 +1754,8 @@
Style.CompactNamespaces = true;
verifyFormat("namespace A { namespace B {\n"
- "}} // namespace A::B",
- Style);
+ "}} // namespace A::B",
+ Style);
EXPECT_EQ("namespace out { namespace in {\n"
"}} // namespace out::in",
@@ -2846,22 +2846,25 @@
EXPECT_EQ(Expected, format(ToFormat, Style));
EXPECT_EQ(Expected, format(Expected, Style));
}
- // Test with tabs.
- Style.UseTab = FormatStyle::UT_Always;
- Style.IndentWidth = 8;
- Style.TabWidth = 8;
- verifyFormat("#ifdef _WIN32\n"
- "#\tdefine A 0\n"
- "#\tifdef VAR2\n"
- "#\t\tdefine B 1\n"
- "#\t\tinclude \n"
- "#\t\tdefine MACRO \\\n"
- "\t\t\tsome_very_long_func_aa();\n"
- "#\tendif\n"
- "#else\n"
- "#\tdefine A 1\n"
- "#endif",
- Style);
+ // Test AfterHash with tabs.
+ {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+ }
// Regression test: Multiline-macro inside include guards.
verifyFormat("#ifndef HEADER_H\n"
@@ -2871,6 +2874,102 @@
" int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ // Basic before hash indent tests
+ verifyFormat("#ifdef _WIN32\n"
+ " #define A 0\n"
+ " #ifdef VAR2\n"
+ "#define B 1\n"
+ "#include \n"
+ "#define MACRO \\\n"
+ " some_very_long_func_aa();\n"
+ " #endif\n"
+ "#else\n"
+ " #define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ " #define MACRO\\\n"
+ "void a(int x) {\\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ "}\n"
+ "#endif",
+ Style);
+ // Keep comments aligned with indented directives. These
+ // tests cannot use verifyFormat because messUp manipulates leading
+ // whitespace.
+ {
+const char *Expected = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ " // Aligned to code.\n"
+ " int a;\n"
+ " #if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ " // Aligned to code.\n"
+ " int b;\n"
+ " #endif\n"
+ "#endif\n"
+ "}";
+const char *ToFormat = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ "// Aligned to code.\n"
+ "int a;\n"
+ "#if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ "// Aligned to code.\n"
+ "int b;\n"
+ "#endif\n"
+ "#endif\n"
+ "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+ }
+ {
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz created this revision.
to-miz added reviewers: djasper, klimek, krasimir, sammccall, mprobst, Nicola.
Herald added a subscriber: cfe-commits.
The option BeforeHash added to IndentPPDirectives.
Fixes Bug 36019.
Repository:
rC Clang
https://reviews.llvm.org/D52150
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1754,8 +1754,8 @@
Style.CompactNamespaces = true;
verifyFormat("namespace A { namespace B {\n"
- "}} // namespace A::B",
- Style);
+ "}} // namespace A::B",
+ Style);
EXPECT_EQ("namespace out { namespace in {\n"
"}} // namespace out::in",
@@ -2846,22 +2846,25 @@
EXPECT_EQ(Expected, format(ToFormat, Style));
EXPECT_EQ(Expected, format(Expected, Style));
}
- // Test with tabs.
- Style.UseTab = FormatStyle::UT_Always;
- Style.IndentWidth = 8;
- Style.TabWidth = 8;
- verifyFormat("#ifdef _WIN32\n"
- "#\tdefine A 0\n"
- "#\tifdef VAR2\n"
- "#\t\tdefine B 1\n"
- "#\t\tinclude \n"
- "#\t\tdefine MACRO \\\n"
- "\t\t\tsome_very_long_func_aa();\n"
- "#\tendif\n"
- "#else\n"
- "#\tdefine A 1\n"
- "#endif",
- Style);
+ // Test AfterHash with tabs.
+ {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+ }
// Regression test: Multiline-macro inside include guards.
verifyFormat("#ifndef HEADER_H\n"
@@ -2871,6 +2874,95 @@
" int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ // Basic before hash indent tests
+ verifyFormat("#ifdef _WIN32\n"
+ " #define A 0\n"
+ " #ifdef VAR2\n"
+ "#define B 1\n"
+ "#include \n"
+ "#define MACRO \\\n"
+ " some_very_long_func_aa();\n"
+ " #endif\n"
+ "#else\n"
+ " #define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ " #define MACRO\\\n"
+ "void a(int x) {\\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ "}\n"
+ "#endif",
+ Style);
+ // Keep comments aligned with indented directives. These
+ // tests cannot use verifyFormat because messUp manipulates leading
+ // whitespace.
+ {
+const char *Expected = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ " // Aligned to code.\n"
+ " int a;\n"
+ " #if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ " // Aligned to code.\n"
+ " int b;\n"
+ " #endif\n"
+ "#endif\n"
+ "}";
+const char *ToFormat = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ "// Aligned to code.\n"
+ "int a;\n"
+ "#if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ "// Aligned to code.\n"
+ "int b;\n"
+ "#endif\n"
+ "#endif\n"
+ "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected,
