[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for bringing this up and implementing it.

I haven't looked into the details of the patch, just some high-level comments:

- the scope of the patch seems a bit unclear to me, what's the problem we are trying to solve?
- the patch looks like calculating the diff/delta highlightings (pre highlightings vs. new highlightings) from a server-only perspective, which seems incorrect. At least we should make sure that LSP client and server share the same understanding of source diff change, otherwise client may render the delta information in a wrong way. To do that, I guess we may start from the `DidChangeTextDocument` notification (where the client sends source changes to server);
- I'm not sure that we should start implement this at the moment -- I don't see that we will get much performance gain, we save some traffic cost between LSP client and server, but at the same time we are spending more CPU resource to compute highlighting diff in server side. Maybe there are some other clever ways (like traversing the decls in source diff change);

We probably need more investigations, like trying the current implementation on some real-project files in a real client (e.g. theia) to figure out and understand whether the latency is a real issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475