[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 30 05:16:51 PDT 2020


MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Format/Format.cpp:2693
+                                           llvm::vfs::FileSystem *FS,
+                                           bool *IsSuitable) {
+  FormatStyle Style = getLLVMStyle();
----------------
pass Style in, don't keep assuming LLVM


================
Comment at: clang/lib/Format/Format.cpp:2694
+                                           bool *IsSuitable) {
+  FormatStyle Style = getLLVMStyle();
+  *IsSuitable = false;
----------------
You cannot keep assuming the style is LLVM, Style needs to be passed in

```
/// Attempts to load a format file
llvm::Expected<FormatStyle> LoadConfigFile(StringRef ConfigFile,
                                           llvm::vfs::FileSystem *FS,
                                           bool *IsSuitable,
                                           FormatStyle Style) {
  *IsSuitable = false;

  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
      FS->getBufferForFile(ConfigFile.str());
  if (std::error_code EC = Text.getError())
    return make_string_error(EC.message());
  std::error_code ParserErrorCode =
      parseConfiguration(Text.get()->getBuffer(), &Style);
  if (ParserErrorCode == ParseError::Unsuitable) {
    return Style;
  } else if (ParserErrorCode != ParseError::Success) {
    return make_string_error("Error reading " + ConfigFile + ": " +
                             ParserErrorCode.message());
  }
  *IsSuitable = true;
  return Style;
}

Now your code will read

```
  // User provided clang-format file using -style=file:/path/to/format/file
  // Check for explicit config filename
  if (StyleName.startswith_lower("file:")) {
    auto StyleNameFile = StyleName.substr(5);
    bool IsSuitable = true;
    auto FileStyle = LoadConfigFile(StyleNameFile, FS, &IsSuitable, Style);
    if (FileStyle && !IsSuitable) {
      return make_string_error("Configuration file(s) do(es) not support " +
                               getLanguageName((*FileStyle).Language) + ": " +
                               StyleNameFile);
    }
    return FileStyle;
  }

```

````
And then in the loop of files you pass the `Style` in too
if (auto ConfigStyle =
                LoadConfigFile(ConfigFile, FS, &IsSuitable, Style)) {
```






================
Comment at: clang/lib/Format/Format.cpp:2743
+    bool IsSuitable = true;
+    auto Style = LoadConfigFile(StyleNameFile, FS, &IsSuitable);
+    if (Style && !IsSuitable) {
----------------
pass Style in and rename return value


================
Comment at: clang/lib/Format/Format.cpp:2763
-        LLVM_DEBUG(llvm::dbgs()
-                   << "Using configuration file " << ConfigFile << "\n");
-        return Style;
----------------
We should probably keep something like this so we know which config file we are loading


================
Comment at: clang/lib/Format/Format.cpp:2785
+        bool IsSuitable;
+        if (auto ConfigStyle = LoadConfigFile(ConfigFile, FS, &IsSuitable)) {
+          if (!IsSuitable) {
----------------
Pass current Style in


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326





More information about the cfe-commits mailing list