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