kbobyrev marked 5 inline comments as done. kbobyrev added inline comments.
================ Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:14 +#include "Index.grpc.pb.h" #include "SourceCode.h" ---------------- sammccall wrote: > this include and the stuff depending on it needs to be ifdef'd Ah, right, I forgot about this. Was present in the previous patch where I had custom definitions, but forgot about it for some reason. ================ Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:40 + RemoteMode("remote", + llvm::cl::desc("Connect to <INDEX LOCATION> remote index")); + ---------------- @sammccall do you know why this opt is not being set for some reason? When I try to run `dexp --help` it is displayed in the possible arguments and when I try to pass anything invalid for boolean flags this also fails, but the value is not changed regardless of the values I put here :( Must be something simple I've missed. ================ Comment at: clang-tools-extra/clangd/index/remote/CMakeLists.txt:1 -generate_grpc_protos(RemoteIndexProtos "Index.proto") +generate_grpc_protos(RemoteIndexProtos "Index.proto" RemoteProtosLocation) ---------------- sammccall wrote: > why is this extra param needed? Because this needs to be included both here and for `dexp` binary. ================ Comment at: clang-tools-extra/clangd/index/remote/Index.proto:16 + + rpc FuzzyFind(FuzzyFindRequest) returns (FuzzyFindReply) {} +} ---------------- sammccall wrote: > should also return a stream. has_more is annoying, but you can keep it in the > message and set it only on the last element of the stream. Is this an idiomatic way of using gRPC + Protobuf? Seems counter-intuitive to me since repeated stream of symbols in the message + single `has_more` seems quite logical. ================ Comment at: clang-tools-extra/clangd/index/remote/Index.proto:37 +message FuzzyFindReply { + // FIXME(kirillbobyrev): Convert to Symbol. + repeated string symbols = 1; ---------------- sammccall wrote: > confused... why not use Symbol now? Couldn't put `Symbol`s into `FuzzyFindReply` for some reason. Clangd behaves really weird with Protobuf inclusions for me... Need to figure out why that happens, but might be me doing something wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78521/new/ https://reviews.llvm.org/D78521 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits