kadircet marked 5 inline comments as done.
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:256
+ diag(Error, "At least one of File or Server must be specified.",
+ llvm::SMRange());
+ return;
----------------
sammccall wrote:
> these missing ranges are potentially pretty difficult for users - they won't
> know e.g. which file the error occurred in.
>
> Do you think we can `Located<>` the whole `ExternalBlock`?
I was doing that initially, but they felt similarly annoying, as they point at
value corresponding to `External` block rather than the whole {key,value}
range. In addition to that it only covers the first line :/
Happy to do that if you think that's still better than an empty/invalid range.
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:305
+
+ // Disable background indexing for the files under the mountpoint.
+ // Note that this will overwrite statements in any previous fragments
----------------
sammccall wrote:
> hang on, you say "under the mountpoint", but... that's not what's happening.
>
> If we want that behavior, we need to use SourceInfo::Directory as the
> mountpoint (and so probably give users a way to set that in the global config
> fragment).
> hang on, you say "under the mountpoint", but... that's not what's happening.
Umm, why? We bail-out at the beginning of apply if filepath doesn't start with
mountpoint ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90749/new/
https://reviews.llvm.org/D90749
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
