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.

Sign up for GitHub

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

Jump to bottom

Add Symbol-search command #689

Merged
merged 1 commit into from
Sep 17, 2021
Merged

Add Symbol-search command #689

merged 1 commit into from
Sep 17, 2021

Conversation

dos65
Copy link
Member

@dos65 dos65 commented Sep 16, 2021

Fixes scalameta/metals#2346.

The build-in symbols search in vscode doesn't work well as it filters out items which label doesn't match query. Beucase of that queries with ; (include dependencies) and short forms like j.n.f.Path haven't worked.

output

@dos65 dos65 requested review from tgodzik and gabro September 16, 2021 20:02
@gabro gabro mentioned this pull request Sep 17, 2021
Closed
gabro
gabro requested changes Sep 17, 2021
View reviewed changes
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

I wish we didn't need to do this, but this is a fair workaround.

Before we merge, I've realized this was never reported properly upstream, so I've just done it here microsoft/vscode#133313

Can you link to that issue in a comment, with a quick explanation of why we have a custom symbol search?

src/extension.ts Outdated Show resolved Hide resolved
@dos65
Copy link
Member Author

dos65 commented Sep 17, 2021

@gabro Actually there is an issue about that. Sorry that I didn't mentioned it - microsoft/vscode#98125 .
According do discussion, it would require changes in protocol and I don't think that it will be fixed soon

@gabro
Copy link
Member

gabro commented Sep 17, 2021

ah, thanks for the pointer, I've marked that issue as duplicate

@dos65 dos65 force-pushed the symbol-search-command branch 2 times, most recently from 914009c to b5b554b Compare September 17, 2021 10:31
@dos65 dos65 requested a review from gabro September 17, 2021 10:32
gabro
gabro requested changes Sep 17, 2021
View reviewed changes
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

Just a minor thing, then 👍

src/extension.ts Show resolved Hide resolved
gabro
gabro reviewed Sep 17, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
gabro
gabro approved these changes Sep 17, 2021
View reviewed changes
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

:shipit:

@gabro
Copy link
Member

gabro commented Sep 17, 2021

@dos65 you may need to rebase to fix the conflict

@dos65 @gabro
Add Symbol-search command
33f51d1 
Fixes [scalameta/metals#2346].
The build-in symbols search in vscode doesn't work well as it filters
out items which label doesn't match query. Beucase of that queries with
`;` (include dependencies) and short forms like `j.n.f.Path` haven't
worked.

Co-authored-by: Gabriele Petronella <gabriele@buildo.io>
@dos65 dos65 merged commit 5f369de into scalameta:main Sep 17, 2021
@dos65 dos65 deleted the symbol-search-command branch September 17, 2021 10:45
@tgodzik
Copy link
Contributor

tgodzik commented Sep 17, 2021

What do you think about releasing it today? I wanted to add the fix for {} and we can just quickly mention that on scalameta.

@gabro
Copy link
Member

gabro commented Sep 17, 2021

👍 sounds good, vscode releases are cheap

@dos65
Copy link
Member Author

dos65 commented Sep 17, 2021

@tgodzik 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabro gabro gabro approved these changes

@tgodzik tgodzik Awaiting requested review from tgodzik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search library dependencies doesn't work
3 participants
@dos65 @gabro @tgodzik