[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file
Zhao Wei Liew via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 27 07:35:54 PST 2021
zwliew added inline comments.
================
Comment at: clang/lib/Format/Format.cpp:3276
+ // Check for explicit config filename
+ if (StyleName.startswith_insensitive("file:")) {
+ auto StyleNameFile = StyleName.substr(5);
----------------
HazardyKnusperkeks wrote:
> 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.
Fair enough. Some ideas off the top of my head that resolve the ambiguity sound needlessly complicated. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72326/new/
https://reviews.llvm.org/D72326
More information about the cfe-commits
mailing list