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

Always interpret the full workspace symbol query as a package name. #2174

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

rgrunber
Copy link
Contributor

  • For a query of 'foo.bar.baz', in addition to searching for all types
    matching 'baz', whose package matches 'foo.bar', we should also search
    for all types whose package matches 'foo.bar.baz'
  • Refactor custom TypeNameMatchRequestor & MethodNameMatchRequestor
  • Add testcase

Signed-off-by: Roland Grunberg [email protected]

This was discussed in redhat-developer/vscode-java#2538 (comment)

CC : @gayanper

@rgrunber rgrunber force-pushed the search-all-types branch 2 times, most recently from e220ed6 to 24e7f6a Compare July 26, 2022 16:46
Copy link
Contributor

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

LGTM

@rgrunber
Copy link
Contributor Author

rgrunber commented Jul 27, 2022

Before merging, I'll try to compare the performance of this feature on queries that produce roughly the same results before & after this change. That way we can know what overhead is introduced by the additional search.

As for using SearchPattern, we would need to confirm to what extent the other logic (regarding working copies) is used as of now in the search and whether we create any problems by changing away from searchAllTypeNames(...). That may take some time but can be done as a secondary step, or in the future if we find ourselves needing to add yet another search criteria.

@gayanper
Copy link
Contributor

That sounds good @rgrunber.

- For a query of 'foo.bar.baz', in addition to searching for all types
  matching 'baz', whose package matches 'foo.bar', we should also search
  for all types whose package matches 'foo.bar.baz'
- Refactor custom TypeNameMatchRequestor & MethodNameMatchRequestor
- Add testcase

Signed-off-by: Roland Grunberg <[email protected]>
@rgrunber
Copy link
Contributor Author

test this please.
(tests failed on 502 error)

@rgrunber
Copy link
Contributor Author

rgrunber commented Aug 2, 2022

test this please.

1 similar comment
@rgrunber
Copy link
Contributor Author

rgrunber commented Aug 2, 2022

test this please.

@rgrunber
Copy link
Contributor Author

rgrunber commented Aug 2, 2022

test this please

@rgrunber rgrunber merged commit 86a86d3 into eclipse-jdtls:master Aug 2, 2022
@rgrunber rgrunber deleted the search-all-types branch August 2, 2022 13:58
@rgrunber rgrunber added the bug label Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants