[PATCH] [clang-format]: Add support for changing case/default indent/outdent with clang-format

2017-02-06 Thread Ryan Livingston via cfe-commits
Hi all,


First time submitting a patch here. Any feedback is appreciated.


This proposed patch adds a new configuration option CaseLabelOffset for 
clang-format which behaves similarly to AccessModifierOffset. Namely, it 
permits modifying the indent/outdent of case and default labels.


With indent level 4, IndentCaseLabels false, and CaseLabelOffset 2 you'll get a 
switch like:


switch (x) {
  case 1:
break;
  case (2):
break;
}

Our team uses this style of formatting and when investigating switching to 
clang-format, I couldn't find a way to accomplish it. So I thought trying a 
patch would be a good route.

For verification, I ran:

  make clang-test

My new tests failed and I iterated running:

  tools/clang/unittests/Format/FormatTests

until everything passed.

Thanks,
Ryan Livingston

Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst	(revision 294126)
+++ docs/ClangFormatStyleOptions.rst	(working copy)
@@ -414,6 +414,10 @@
 **BreakStringLiterals** (``bool``)
   Allow breaking string literals when formatting.
 
+**CaseLabelOffset** (``int``)
+  The extra indent or outdent of case/default labels
+  e.g. ``case:``, ``default:``.
+
 **ColumnLimit** (``unsigned``)
   The column limit.
 
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h	(revision 294126)
+++ include/clang/Format/Format.h	(working copy)
@@ -292,6 +292,10 @@
   /// \brief Allow breaking string literals when formatting.
   bool BreakStringLiterals;
 
+  /// \brief The extra indent or outdent of case/default labels
+  /// e.g. ``case:``, ``default:``.
+  int CaseLabelOffset;
+
   /// \brief The column limit.
   ///
   /// A column limit of ``0`` means that there is no column limit. In this case,
@@ -667,6 +671,7 @@
R.BreakConstructorInitializersBeforeComma &&
BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
BreakStringLiterals == R.BreakStringLiterals &&
+   CaseLabelOffset == R.CaseLabelOffset &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp	(revision 294126)
+++ lib/Format/Format.cpp	(working copy)
@@ -295,6 +295,7 @@
 IO.mapOptional("BreakAfterJavaFieldAnnotations",
Style.BreakAfterJavaFieldAnnotations);
 IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
+IO.mapOptional("CaseLabelOffset", Style.CaseLabelOffset);
 IO.mapOptional("ColumnLimit", Style.ColumnLimit);
 IO.mapOptional("CommentPragmas", Style.CommentPragmas);
 IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
@@ -520,6 +521,7 @@
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.BreakStringLiterals = true;
+  LLVMStyle.CaseLabelOffset = 0;  
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp	(revision 294126)
+++ lib/Format/UnwrappedLineFormatter.cpp	(working copy)
@@ -94,6 +94,8 @@
 (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
  RootToken.Next && RootToken.Next->is(tok::colon)))
   return Style.AccessModifierOffset;
+if (RootToken.isOneOf(tok::kw_case, tok::kw_default))
+  return Style.CaseLabelOffset;
 return 0;
   }
 
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp	(revision 294126)
+++ unittests/Format/FormatTest.cpp	(working copy)
@@ -778,6 +778,20 @@
getLLVMStyleWithColumns(34));
 }
 
+TEST_F(FormatTest, CaseOffset) {
+  FormatStyle Style = getLLVMStyle();
+  Style.CaseLabelOffset = 2;
+  Style.IndentCaseLabels = false;
+  Style.IndentWidth = 4;
+  verifyFormat("switch (x) {\n"
+   "  case 1:\n"
+   "break;\n"
+   "  case (2):\n"
+   "break;\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTest, CaseRanges) {
   verifyFormat("switch (x) {\n"
"case 'A' ... 'Z':\n"
@@ -10367,6 +10381,7 @@
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset, -1234);
+  CHECK_PARSE("CaseLabelOffset: -1235", CaseLabelOffset, -1235);
   CHECK_PARSE("ConstructorInitializerIndentWidth: 1234",
   ConstructorInitializerI

Re: [PATCH] [clang-format]: Add support for changing case/default indent/outdent with clang-format

2017-02-07 Thread Ryan Livingston via cfe-commits
When adding more tests, an issue with this approach was found using scopes:


switch (x) {
  case 1:
break;
  case (2):
break;
  case 3: {
break;
} // <-- Seems we should offset this by 2
}

I'll look into getting this behavior and update the patch.

~Ryan



From: Ryan Livingston
Sent: Monday, February 6, 2017 7:28:54 AM
To: cfe-commits@lists.llvm.org
Subject: [PATCH] [clang-format]: Add support for changing case/default 
indent/outdent with clang-format


Hi all,


First time submitting a patch here. Any feedback is appreciated.


This proposed patch adds a new configuration option CaseLabelOffset for 
clang-format which behaves similarly to AccessModifierOffset. Namely, it 
permits modifying the indent/outdent of case and default labels.


With indent level 4, IndentCaseLabels false, and CaseLabelOffset 2 you'll get a 
switch like:


switch (x) {
  case 1:
break;
  case (2):
break;
}

Our team uses this style of formatting and when investigating switching to 
clang-format, I couldn't find a way to accomplish it. So I thought trying a 
patch would be a good route.

For verification, I ran:

  make clang-test

My new tests failed and I iterated running:

  tools/clang/unittests/Format/FormatTests

until everything passed.

Thanks,
Ryan Livingston

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits