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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits