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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:3393
   if (!ChildFormatTextToApply.empty()) {
     assert(ChildFormatTextToApply.size() == 1);
 
----------------
zwliew wrote:
> Is there a reason for this to be limited to 1? I came across this case during testing, but I can't think of why this should be so.
See comment on D93844.


================
Comment at: clang/lib/Format/Format.cpp:3276
+  // Check for explicit config filename
+  if (StyleName.startswith_insensitive("file:")) {
+    auto StyleNameFile = StyleName.substr(5);
----------------
zwliew wrote:
> Following my code suggestion above, I think this code block could be refactored to this: 
> ```
> // Check for explicit config filename
> if (StyleName.startswith_insensitive("file:")) {
>     auto StyleFileName = StyleName.substr(5);
>     if (auto ec = loadConfigFile(StyleFileName, FS, &Style)) {
>         return make_string_error(ec.message());
>     }
>     LLVM_DEBUG(llvm::dbgs() << "Using configuration file " << StyleFileName << '\n');
>     return Style;
> }
> ```
> 
> What do you think?
> 
> Also, on another note, I feel like the `-style` option is too overloaded. Would it be better to use a separate option like `-style-file` instead?
You can certainly refactor code.

What happens with `-style="Google" -style-file="Foo/Bar"` is it different from `-style-file="Foo/Bar" -style="Google"`?
I do not perse disagree, but I think it makes stuff clearer if there is only one option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72326