ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D53688#1275793, @sammccall wrote:

> It's hard to reason about UX outside of concrete tools - the only use I know 
> of (atom-ide-cpp) never needs to change it, and I'm having a hard time 
> imagining when you'd want to change this, but still have it global to clangd. 
> (I can imagine it being scoped to a subtree a la compile commands, but that's 
> a whole different thing..)
>
> And yes, there are significant implementation concerns with making it 
> mutable: ultimately we're going to want to have a CDB -> clangd compile 
> command invalidation mechanism that can drive reindexing, reloading 
> diagnostics etc. If the fallback command is mutable, such a mechanism needs 
> to handle wildcards (or accept that this is a case it will get wrong, there 
> are others).


Whatever the tool is, the choices for it are to allow changing some 
configuration at the init time or while running. Just realized that we're 
removing the compileCommandsDir for that specific reason, sorry for being out 
of context here.

The atom-ide-cpp use case is somewhat special anyway, since it ends up reading 
a file in a format we already support, albeit having a different name from the 
one clangd would expect (correct me if I'm wrong on this one).
In the long-run it seems better to communicate to the users that they should 
rename the file to make clangd work rather than add this (a bit different and 
more powerful?) extension to clangd.

We're aiming to allow users change their compile flags while running clangd 
anyway (assuming we're gonna get the file watches in at some point), so, again, 
in the long-run I don't see why that would complicate things. But sympathetic 
to keeping the code simpler anyway.

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53688



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to