sammccall added a comment.

I'm not sure we should do this - it complicates the model substantially (I 
don't think I could explain it to a user), and it still leaves surprising cases.

I'm not sure config is the place for a lot of magic do-what-I-mean behavior - 
it's the place where the user overrides such behaviors.

(If you think the combinations are too confusing, we could consider *not* 
disabling background index implicitly for external indexes, and simply warning 
instead)



================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:306
+          // Disable background indexing for the files under the mountpoint. If
+          // user didn't take any explicit actions.
+          if (C.Index.Background == Config::BackgroundPolicy::Auto)
----------------
This is vague and hard to specify more precisely.


================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:308
+          if (C.Index.Background == Config::BackgroundPolicy::Auto)
+            C.Index.Background = Config::BackgroundPolicy::Skip;
         });
----------------
doesn't this mean that if a user has explicit `Background: Build` at a higher 
scope, then a more narrowly scoped external index won't disable it?

This seems at least as confusing as the problems this is trying to solve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90775

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

Reply via email to