- User Since
- Aug 26 2016, 6:53 AM (111 w, 4 d)
Rebase to include CodeAction literal support
Addressed review comments.
Also inverted the sense of the boolean return from onNotify etc, was "should
exit" and is now "should continue", which I think is slightly clearer.
(please do check there are no duplicates in the output)
This looks reasonable. +80% to index size is a shame, but not disastrous, and we have ways to shave it.
Add unit test for JSON transport.
Works for me! I think the code can be simplified slightly.
The slightly weird down-traversals semantics are indeed slightly weird. I'm happy to move forward with this approach though if we document it.
I think it'd be a good idea to separate out the on-initialization vs dynamically-changing parameters more - I think they should probably be disjoint in fact.
For reference, @jkorous has a WIP in D53290.
It's scope is a superset, and I think everything in common is basically the same (they were both based on a common prototype).
Jan is out at the moment, so I think it makes sense to move ahead with this piece - it's progress, and the smaller scope makes landing the change easier.
Address comments, strip out of clangdlspserver for now.
This patch is big and hard to navigate. I tried to keep it contained, but JSONRPCDispatcher is pretty tangled.
Nice fix, just performance concerns.
Sun, Oct 14
(This looks good, of course the Sema patch needs to land first!)
Thanks for cleaning this up!
Fri, Oct 12
We could verify the fuzzy-matched results, but we have pretty good testing on the C++ side, so a smoke test here seems fine.
Oops, forgot one comment.
Address comments, add tests.
Some changes (enqueue(), blockUntilIdleForTest()) to support testing.
(BTW I wouldn't expect to see improvements on eval here, as
Thu, Oct 11
Sorry about that. I wasn't familiar with the python bindings.
Your bisect is correct, we changed the behavior that test is testing: now
an approximate match will be returned.
Agree with you both here. Not being able to reset the initial state is weird, but *being able to* is complicated. (Even more so than switching between CDBs).
And since switching between CDBs invalidates almost everything, restarting clangd seems like a great simplification if it's viable for you.
Supporting a single CDB that's available on startup (either a flag, or maybe more likely set during initialize()?) would be a very useful simplification. (Even if it's not a lot of code, I find myself getting tangled in it often).
Wed, Oct 10
Tue, Oct 9
Mon, Oct 8
Hmm, I wonder if we should remove "experimental" text and the hidden bit. Or wait to see if we want to move to automatic index entirely?
May want to mention "must have been created by a compatible clangd-indexer".