Hi djasper, klimek,

+ improved handling of default style and predefined styles.

http://llvm-reviews.chandlerc.com/D813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/FormatTest.cpp
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -161,11 +161,13 @@
 /// https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style.
 FormatStyle getMozillaStyle();
 
-/// \brief Returns a predefined style by name.
+/// \brief Gets a predefined style by name.
 ///
 /// Currently supported names: LLVM, Google, Chromium, Mozilla. Names are
 /// compared case-insensitively.
-FormatStyle getPredefinedStyle(StringRef Name);
+///
+/// Returns true if the Style has been set.
+bool getPredefinedStyle(StringRef Name, FormatStyle *Style);
 
 /// \brief Parse configuration from YAML-formatted text.
 llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -60,16 +60,21 @@
       ArrayRef<StringRef> Styles(StylesArray);
       for (size_t i = 0, e = Styles.size(); i < e; ++i) {
         StringRef StyleName(Styles[i]);
-        if (Style == clang::format::getPredefinedStyle(StyleName)) {
+        clang::format::FormatStyle PredefinedStyle;
+        if (clang::format::getPredefinedStyle(StyleName, &PredefinedStyle) &&
+            Style == PredefinedStyle) {
           IO.mapOptional("# BasedOnStyle", StyleName);
           break;
         }
       }
     } else {
       StringRef BasedOnStyle;
       IO.mapOptional("BasedOnStyle", BasedOnStyle);
       if (!BasedOnStyle.empty())
-        Style = clang::format::getPredefinedStyle(BasedOnStyle);
+        if (!clang::format::getPredefinedStyle(BasedOnStyle, &Style)) {
+          IO.setError(Twine("Unknown value for BasedOnStyle: ", BasedOnStyle));
+          return;
+        }
     }
 
     IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
@@ -180,18 +185,19 @@
   return MozillaStyle;
 }
 
-FormatStyle getPredefinedStyle(StringRef Name) {
+bool getPredefinedStyle(StringRef Name, FormatStyle *Style) {
   if (Name.equals_lower("llvm"))
-    return getLLVMStyle();
-  if (Name.equals_lower("chromium"))
-    return getChromiumStyle();
-  if (Name.equals_lower("mozilla"))
-    return getMozillaStyle();
-  if (Name.equals_lower("google"))
-    return getGoogleStyle();
+    *Style = getLLVMStyle();
+  else if (Name.equals_lower("chromium"))
+    *Style = getChromiumStyle();
+  else if (Name.equals_lower("mozilla"))
+    *Style = getMozillaStyle();
+  else if (Name.equals_lower("google"))
+    *Style = getGoogleStyle();
+  else
+    return false;
 
-  llvm::errs() << "Unknown style " << Name << ", using Google style.\n";
-  return getGoogleStyle();
+  return true;
 }
 
 llvm::error_code parseConfiguration(StringRef Text, FormatStyle *Style) {
Index: tools/clang-format/ClangFormat.cpp
===================================================================
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -27,6 +27,9 @@
 
 using namespace llvm;
 
+// Default style to use when no style specified or specified style not found.
+static const char *DefaultStyle = "LLVM";
+
 static cl::opt<bool> Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
 // Mark all our options with this category, everything else (except for -version
@@ -54,11 +57,14 @@
     Style("style",
           cl::desc("Coding style, currently supports:\n"
                    "  LLVM, Google, Chromium, Mozilla.\n"
-                   "Use '-style file' to load style configuration from\n"
+                   "Use -style=file to load style configuration from\n"
                    ".clang-format file located in one of the parent\n"
                    "directories of the source file (or current\n"
-                   "directory for stdin)."),
-          cl::init("LLVM"), cl::cat(ClangFormatCategory));
+                   "directory for stdin).\n"
+                   "Use -style=\"{key: value, ...}\" to set specific\n"
+                   "parameters, e.g.:\n"
+                   "  -style=\"{BasedOnStyle: llvm, IndentWidth: 8}\""),
+          cl::init(DefaultStyle), cl::cat(ClangFormatCategory));
 static cl::opt<bool> Inplace("i",
                              cl::desc("Inplace edit <file>s, if specified."),
                              cl::cat(ClangFormatCategory));
@@ -88,8 +94,24 @@
 }
 
 FormatStyle getStyle(StringRef StyleName, StringRef FileName) {
-  if (!StyleName.equals_lower("file"))
-    return getPredefinedStyle(StyleName);
+  FormatStyle Style;
+  getPredefinedStyle(DefaultStyle, &Style);
+
+  if (StyleName.startswith("{")) {
+    // Parse YAML/JSON style from the command line.
+    if (error_code ec = parseConfiguration(StyleName, &Style)) {
+      llvm::errs() << "Error parsing -style: " << ec.message()
+                   << ", using " << DefaultStyle << " style\n";
+    }
+    return Style;
+  }
+
+  if (!StyleName.equals_lower("file")) {
+    if (!getPredefinedStyle(StyleName, &Style))
+      llvm::errs() << "Invalid value for -style, using " << DefaultStyle
+                   << " style\n";
+    return Style;
+  }
 
   SmallString<128> Path(FileName);
   llvm::sys::fs::make_absolute(Path);
@@ -109,7 +131,6 @@
         llvm::errs() << ec.message() << "\n";
         continue;
       }
-      FormatStyle Style;
       if (error_code ec = parseConfiguration(Text->getBuffer(), &Style)) {
         llvm::errs() << "Error reading " << ConfigFile << ": " << ec.message()
                      << "\n";
@@ -119,8 +140,9 @@
       return Style;
     }
   }
-  llvm::errs() << "Can't find usable .clang-format, using LLVM style\n";
-  return getLLVMStyle();
+  llvm::errs() << "Can't find usable .clang-format, using " << DefaultStyle
+               << " style\n";
+  return Style;
 }
 
 // Returns true on error.
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4209,27 +4209,27 @@
 }
 
 TEST_F(FormatTest, GetsPredefinedStyleByName) {
-  FormatStyle LLVMStyles[] = { getLLVMStyle(), getPredefinedStyle("LLVM"),
-                               getPredefinedStyle("llvm"),
-                               getPredefinedStyle("lLvM") };
-  EXPECT_TRUE(allStylesEqual(LLVMStyles));
-
-  FormatStyle GoogleStyles[] = { getGoogleStyle(), getPredefinedStyle("Google"),
-                                 getPredefinedStyle("google"),
-                                 getPredefinedStyle("gOOgle") };
-  EXPECT_TRUE(allStylesEqual(GoogleStyles));
-
-  FormatStyle ChromiumStyles[] = { getChromiumStyle(),
-                                   getPredefinedStyle("Chromium"),
-                                   getPredefinedStyle("chromium"),
-                                   getPredefinedStyle("chROmiUM") };
-  EXPECT_TRUE(allStylesEqual(ChromiumStyles));
-
-  FormatStyle MozillaStyles[] = { getMozillaStyle(),
-                                  getPredefinedStyle("Mozilla"),
-                                  getPredefinedStyle("mozilla"),
-                                  getPredefinedStyle("moZilla") };
-  EXPECT_TRUE(allStylesEqual(MozillaStyles));
+  FormatStyle Styles[3];
+
+  Styles[0] = getLLVMStyle();
+  getPredefinedStyle("LLVM", &Styles[1]);
+  getPredefinedStyle("lLvM", &Styles[2]);
+  EXPECT_TRUE(allStylesEqual(Styles));
+
+  Styles[0] = getGoogleStyle();
+  getPredefinedStyle("Google", &Styles[1]);
+  getPredefinedStyle("gOOgle", &Styles[2]);
+  EXPECT_TRUE(allStylesEqual(Styles));
+
+  Styles[0] = getChromiumStyle();
+  getPredefinedStyle("Chromium", &Styles[1]);
+  getPredefinedStyle("cHRoMiUM", &Styles[2]);
+  EXPECT_TRUE(allStylesEqual(Styles));
+
+  Styles[0] = getMozillaStyle();
+  getPredefinedStyle("Mozilla", &Styles[1]);
+  getPredefinedStyle("moZILla", &Styles[2]);
+  EXPECT_TRUE(allStylesEqual(Styles));
 }
 
 TEST_F(FormatTest, ParsesConfiguration) {
@@ -4242,7 +4242,7 @@
 #define CHECK_PARSE_BOOL(FIELD)                                                \
   Style.FIELD = false;                                                         \
   EXPECT_EQ(0, parseConfiguration(#FIELD ": true", &Style).value());           \
-  EXPECT_TRUE(Style.FIELD);                                                \
+  EXPECT_TRUE(Style.FIELD);                                                    \
   EXPECT_EQ(0, parseConfiguration(#FIELD ": false", &Style).value());          \
   EXPECT_FALSE(Style.FIELD);
 
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to