[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