Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3252 Add support for scoping the search operations #3253

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gayanper
Copy link
Contributor

The current version support scoping the following search operations

  • reference search
  • call hierarchy search

with the following scopes to choose from

  • all : includes all classpath entries
  • main: all classpath entries excluding test

@rgrunber
Copy link
Contributor

rgrunber commented Aug 26, 2024

Seems very reasonable. I guess we don't have to add it to

private IJavaSearchScope createSearchScope() throws JavaModelException {
IJavaProject[] projects = JavaCore.create(ResourcesPlugin.getWorkspace().getRoot()).getJavaProjects();
return SearchEngine.createJavaSearchScope(projects, IJavaSearchScope.SOURCES);
}
because that only deals with source references ? So the number of references in the references code lens will still match what the references view shows. What about workspace symbol search
int scope = IJavaSearchScope.REFERENCED_PROJECTS | IJavaSearchScope.SOURCES;
PreferenceManager preferenceManager = JavaLanguageServerPlugin.getPreferencesManager();
if (!sourceOnly && preferenceManager != null && preferenceManager.isClientSupportsClassFileContent()) {
scope |= IJavaSearchScope.APPLICATION_LIBRARIES | IJavaSearchScope.SYSTEM_LIBRARIES;
}
return SearchEngine.createJavaSearchScope(targetProjects, scope);
?

@rgrunber rgrunber added this to the End September 2024 milestone Aug 27, 2024
@gayanper
Copy link
Contributor Author

Seems very reasonable. I guess we don't have to add it to

private IJavaSearchScope createSearchScope() throws JavaModelException {
IJavaProject[] projects = JavaCore.create(ResourcesPlugin.getWorkspace().getRoot()).getJavaProjects();
return SearchEngine.createJavaSearchScope(projects, IJavaSearchScope.SOURCES);
}

because that only deals with source references ? So the number of references in the references code lens will still match what the references view shows. What about workspace symbol search

int scope = IJavaSearchScope.REFERENCED_PROJECTS | IJavaSearchScope.SOURCES;
PreferenceManager preferenceManager = JavaLanguageServerPlugin.getPreferencesManager();
if (!sourceOnly && preferenceManager != null && preferenceManager.isClientSupportsClassFileContent()) {
scope |= IJavaSearchScope.APPLICATION_LIBRARIES | IJavaSearchScope.SYSTEM_LIBRARIES;
}
return SearchEngine.createJavaSearchScope(targetProjects, scope);

?

Good point, I think we should add there as well, because

  • IJavaSearchScope.SOURCES : This consider both main and test code as well isn't it ?
  • WorkspaceSymbols : I think we should add there as well, so that it is much straight forward for user when they switch the scope.

WDYT ?

@rgrunber rgrunber removed this from the End September 2024 milestone Sep 5, 2024
@rgrunber
Copy link
Contributor

rgrunber commented Sep 5, 2024

You're right :

With main set, the references view correctly shows 1 reference while code lens references still show 2 (1 coming from a test folder).
image

We definitely need to adjust CodeLensHandler. I guess I must have thought that SOURCES meant just things part of the runtime but that's wrong. It clearly includes test references.

Agreed on WorkspaceSymbols.

@gayanper
Copy link
Contributor Author

gayanper commented Sep 8, 2024

We definitely need to adjust CodeLensHandler. I guess I must have thought that SOURCES meant just things part of the runtime but that's wrong. It clearly includes test references.

Add support for code lens reference.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine overall. What about having the workspace symbol search respect this search scope as well ?

I could see this being useful as a bunch of times, I find myself wishing I could exclude everything with 'test' in it when I do a generic text search.

The current version support scoping the following search operations
- reference search
- call hierarchy search

with the following scopes to choose from
- all : includes all classpath entries
- main: all classpath entries excluding test
@gayanper
Copy link
Contributor Author

bad habit :( , I rebase against master and force push the changes due to target file outdated. Hope it will be ok with you, my new changes on workspace symbols are on a new commit though.

@rgrunber rgrunber added this to the End September 2024 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants