-
Notifications
You must be signed in to change notification settings - Fork 162
Add Just My Code support for global symbol search #823
base: master
Are you sure you want to change the base?
Conversation
@@ -232,13 +232,21 @@ struct Config { | |||
}; | |||
Index index; | |||
|
|||
// List of folders opened in current workspace. | |||
std::vector<std::string> workspaceFolders; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably shouldn't be in config, can you store it as a variable in the base message handler class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm..
Don't know how to achieve this. Okay, let me put forth my understanding of LSP.
- There are only two ways to receive config options from clients.
- Through initialization options - stored in
struct Config
. - Through --init command line option - can be stored in
strut MessageHandler
or in one of its fields.
- Through initialization options - stored in
- Clients don't (can't) send extra options through queries.
Please, correct me if I'm wrong regarding these assumptions.
Coming to the change you suggested, did you mean struct MessageHandler
or struct BaseMessageHandler
.
In either case please elaborate on how to achieve this. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In MessageHandler
, ie, where db
, project
and the like are defined (src/message_handler.h line 85ish). You can declare a std::unique_ptr<std::vector<std::string>> workspaceFolders
there and append to it inside of init, ie, so that every derived MessageHandler type will use the same std::vector
instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if you don't feel like doing that, a TODO is good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, for clarification. I've made the changes you suggested. Check if that is what you intended.
Please feel free to suggest further changes.
Thanks! This seems like a nice improvement, what about we just make this standard behavior and add an option to disable the automatic filtering in settings? |
Just my code option filters all non workspace symbols returned to client