[PATCH] D145843: [clangd] Add option to always insert headers with <> instead of ""

Tue Mar 21 07:59:47 PDT 2023

sammccall added a comment.

In D145843#4207466 <https://reviews.llvm.org/D145843#4207466>, @kadircet wrote:

> In D145843#4207101 <https://reviews.llvm.org/D145843#4207101>, @nridge wrote:
>
>> My understanding is that a more elaborate configuration scheme has been proposed in https://github.com/clangd/clangd/issues/1367, and the feedback there was (quoting Sam from this comment <https://github.com/clangd/clangd/issues/1367#issuecomment-1322018307>):
>> The approach taken in this patch seemed to me to be in line with this direction of a "simple config-based solution".
>
> I am afraid this approach is a little "too simple". The intent on @sammccall 's comment there is probably around getting rid of the re-writing of include spellings completely, not for specifying quoted or system includes based on the include spelling.
> Even if not, I feel like my argument above still applies. I still can't think of many projects benefiting from always using angles/quotes for include spellings.

Right - my impression is that most projects that want to force a certain quote style want to do so for certain paths only.

e.g. from #1367:

> I'm working on a project where the style dictates that all the includes **from our public SDK** are included using angle brackets.

A recent comment on #1247:

> Since clangd works with little configuring for SerenityOS, we do not need to modify -isystem which is picked up from our toolchain. However, those includes do not include all **the headers we include with the bracketed style**, so a better way of configuring this is needed.

So while I do think it's important to limit the complexity, a version of the feature that isn't actually usable to most people would just be an attractive nuisance.

I'd suggest something like:

  Style:
    QuotedHeaders: "path/to/.*"
    AngledHeaders: "path/sdk/.*"

where the regexes can be single or array, and match any suffix (like Diagnostics.Includes.IgnoreHeader)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145843