Small improvements to workspace/symbol
#5443
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Duhemm
commented
Nov 14, 2018
Duhemm
commented
- Inspect only source trees
- Exclude local symbols
We were inspecting all trees on the classpath for every project, which was not efficient and lead to duplicate results. This commit changes the implementation so that the language server inspects only the source trees of every project.
Duhemm
force-pushed
the
ide-workspace-symbols
branch
from
ca776a7 to
7fd0c1d
Compare
|
Rebased |
|
@smarter can you take a look? |
smarter
approved these changes
Nov 28, 2018
| @@ -466,12 +471,14 @@ class DottyLanguageServer extends LanguageServer | |||
|
|
|||
| override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken => | |||
| val query = params.getQuery | |||
| def predicate(implicit ctx: Context): NameTree => Boolean = | |||
| tree => tree.symbol.exists && !tree.symbol.isLocal && tree.name.toString.contains(query) | |||
There was a problem hiding this comment.
Shouldn't symbol and documentSymbol have similar predicates ? I.e., should we also exclude primary constructors and synthetic symbols here ?
There was a problem hiding this comment.
Yes, that makes a lot of sense. I've removed checks that were redundant with those happening in namedTrees.
I'm also thinking that namedTrees should be the one excluding primary constructors. I can't think of situations where they should be included, but I may very well be missing something. What do you think?
Some checks were redundant with the checks that are performed in `namedTrees`, and can be removed. Also, exclude primary constructors in `symbol`.
|
Agreed, as long as go to definition on a new call still goes to the correct place |
We never want to match them, so it's easier to exclude them here rather than always add a check in the predicate function.
smarter
reviewed
Nov 29, 2018
language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala
Outdated
Show resolved
Hide resolved
smarter
approved these changes
Nov 29, 2018
This include flag means that local symbols and trees should be inspected.
Duhemm
force-pushed
the
ide-workspace-symbols
branch
from
6f141db to
8213555
Compare
👍 |
smarter
approved these changes
Nov 29, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.