[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added a comment. Sorry if I missed any important design discussions here, but wanted to clear up what information we are trying to convey to the user with the status messages? E.g. why seeing "building preamble", "building file" or "queued" in the status bar can be useful to the us

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

simark added a comment. In D55363#1329652 , @ilya-biryukov wrote: > Sorry if I missed any important design discussions here, but wanted to clear > up what information we are trying to convey to the user with the status > messages? > E.g. why seeing "bu

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added a comment. Good point, we should definitely state it's a clangd-specific extension. We could come up with a naming scheme for our extensions. I would propose something like `clangd:$methodName`, i.e. the current extensions would be `clangd:textDocument/fileStatus`. WDYT? R

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 178455. hokein added a comment. Update: - avoid using terms that C++ programmers are unfamiliar with. - textDocument/fileStatus => textDocument/clangd.fileStatus Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5536

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein added a comment. > Sorry if I missed any important design discussions here, but wanted to clear > up what information we are trying to convey to the user with the status > messages? > E.g. why seeing "building preamble", "building file" or "queued" in the > status bar can be useful to t

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added a comment. Could we add a capability to the init request to check for this extension? We don't want to send those notifications to clients that don't have the code to handle them. Another observation: when I played around with the previous version of the patch, `RunningActi

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 179041. hokein marked 3 inline comments as done. hokein added a comment. address review comments. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55363/new/ https://reviews.llvm.org/D55363 Files: clangd/ClangdLS

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein added a comment. In D55363#1336315 , @ilya-biryukov wrote: > Could we add a capability to the init request to check for this extension? We > don't want to send those notifications to clients that don't have the code to > handle them. yes, I thi

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added inline comments. Comment at: clangd/Protocol.cpp:769 O.map("fallbackFlags", Opts.fallbackFlags); + O.map("fileStatus", Opts.FileStatus); return true; Ah, there's a non-zero chance of name clash here in case the protocol implements some

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 179046. hokein marked 4 inline comments as done. hokein added a comment. Remove details. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55363/new/ https://reviews.llvm.org/D55363 Files: clangd/ClangdLSPServer.c

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 179047. hokein added a comment. Rebase Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55363/new/ https://reviews.llvm.org/D55363 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/Protocol.cp

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein added inline comments. Comment at: clangd/index/Merge.cpp:137 S.ReturnType = O.ReturnType; + if (S.Type == "") +S.Type = O.Type; ilya-biryukov wrote: > Accidental change? oops, I rebased this branch on top of my another branch. Fixed. Repositor

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55363/new/ https://reviews.llvm.org/D55363 __

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349768: [clangd] Expose FileStatus to LSP. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D55363?vs=179047&id=179080#toc Repository: rCTE Clang Tools Extra

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

jkorous added a comment. In D55363#1337376 , @hokein wrote: > In D55363#1336315 , @ilya-biryukov > wrote: > > > Maybe consider sending an update after some period of time, e.g. `0.5s`? (I > > expect this particula

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

ilya-biryukov added a comment. In D55363#1343611 , @jkorous wrote: > I am a bit late to the discussion but isn't this something to be dealt with > on the client's side? That would mean more complex code in **every** client and more communication betwee

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, javed.absar. Add an LSP extension "textDocument/fileStatus" to emit file-status information. Repository: rCTE Clang Tools Extra https://reviews.llvm.o

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

hokein updated this revision to Diff 176983. hokein added a comment. Update. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55363/new/ https://reviews.llvm.org/D55363 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/Protocol.c