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

Fri Jun 7 03:14:29 PDT 2019

Typz added a comment.

In D50147#1533337 <https://reviews.llvm.org/D50147#1533337>, @sammccall wrote:

> One thing that's unclear to me is whether your aim is to
>
> 1. solve a concrete problem for your organization
> 2. solve a family of problems for similar organizations
> 3. add a new way of configuring styles for many types of users/projects

First and forehand, I have a problem to solve in my organization : we have many projects, and maintaining the config file in some many repositories is not practical.
In some case (like, LLVM, google or mozilla), this is not an issue because the formatting rules are hard-coded in clang-format.
In other large organisation (like Qt, KDE or many private compagnies), there may be coding-rules already in place and it is not practical to store the configuration in clang-format.
(This is not so much an issue in small-organisation, with few projects/repositories : each project/repo may manage its own clang-format config file)

The goal is to support this use-case : organisation-wide styles, without patching clang-format.

> If it's 1) I think this is very reasonable and we should focus on finding the simplest mechanism that will work. Is it possible to always set an environment variable, or a build-time parameter, or install the style file in a well-known absolute path (is there really a mix of win/unix users?)

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.
Therefore, I think the solution should still be generic enough, and there is no reason to restrict it to one use case.

> If it's 2), I think what's missing is evidence that other orgs have similar problems and requirements.

I think many orgs have the same issue, but some of them have found a solution by hard-coding their style in clang-format...

> Option 3) is interesting, but much more ambitious and needs to start with a design doc that discusses the use cases, alternatives, and implications. Certainly anything involving fetching styles via git/http falls into this bucket, I think.

git/http/... is indeed much more complicated. It would solve the issue of "transparent" setup, but introduces many subtleties : how to cache the result (to handle offline use or downtime issues), how to handle updates....

> @klimek in case I'm way off base here, or there have been past discussions that shed light on this.
> 
> With that in mind, I'd be very happy to approve the build time config and/or an env variable, as long as they're off by default. It's easy to turn them on later, but not easy to turn them off.
>  If they're going to be on by default, I think we need a strong reason.

I they are going to be off by default, it means we would still need to patch clang-format to use it, correct ?
I would like to avoid this, as it is quite time consuming; and it would actually be easier in that case to just add yet another hard-coded style in the patched binary...

In D50147#1533337 <https://reviews.llvm.org/D50147#1533337>, @sammccall wrote:

> >> - understanding how distro packaging is going to work
>
> There's a mechanism, but how is it to be used? Will/should projects with a style guide provide style packages for distros? Or should these be part of the "official" clang-format package? 
>  If separate packages exist, how much is it going to confuse users that clang-format will silently format the same project with a `.clang-format` file different ways depending on what's installed?

The goal is to actually separate the styles from clang-format : so I don't see the point to make them part of the official clang-format package.
Usage may be different: the styles may be setup through different packages (e.g. Qt style in qt-core package), installed manually by user, ....
This is surely not perfect, since different packages may indeed provide the same style : technically this is not an issue (packages must just be marked as conflicting), but it is indeed the organisation's responsibility to use different names for the styles...
(to some extent, this may be improved by passing URLs for external styles, e.g. from git ; but this may even be an issue if different mirrors must be used...)

Repository:
  rC Clang

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

https://reviews.llvm.org/D50147