[PATCH] D50147: clang-format: support external styles

Wed Nov 20 03:50:49 PST 2019

sammccall added a comment.

In D50147#1751198 <https://reviews.llvm.org/D50147#1751198>, @Typz wrote:

> >> In our case, we actually have more than one "standard" style, a combination of various OS (linux, windows, macos), and not a very strong control on user computers. So we cannot rely on a specific file or variable being setup by an administrator.
> > 
> > In this case my best advice would be in the short term to use .clang-format files. Longer term, some combination of using well-known styles, publicising and teaching clang-format about the styles you use, and gaining the ability to set environment variables would reduce duplication.
>
> what do you mean about "gaining the ability to set environment variables" ?

If you can set a CLANG_FORMAT_STYLE_PATH or so environment variable, then you can install your styles there

> - Build option is implemented. This allows turn the feature off if needed, at build time (by specifying empty search path). I would prefer to keep thos
> - Overriding an environment varialbe to change the search path is fine by me. But I would still prefer to have a "working" default, so that it can be used out-of-the-box, with no extra env variable to set

OK, I'm not going to approve turning this feature on by default (specifically: searching unknown strings as paths relative to some directory baked into the binary). I think it's bad for the ecosystem.
If you want to use absolute paths, or set the path with a build option, environment variable, or flag that seems fine to me.

> The goal is indeed that user keep installing clang-format through  LLVM releases or OS distributors; but they would also install the custom styles they need, which would be provided by their organization (not clang/llvm!): e.g. Qt style, or "my-compagny" style.

My concern about the ecosystem is largely about requiring users to know which styles they need.

Repository:
  rC Clang

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

https://reviews.llvm.org/D50147