Skip to content

Allow to query workspace/symbols by a SymbolDescriptor #12

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

Merged
merged 3 commits into from
Jan 17, 2017
Merged

Allow to query workspace/symbols by a SymbolDescriptor #12

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7b2d24
Allow to query symbols by SymbolDescriptor
felixfbecker Jan 15, 2017
5c456fd
Add comparison between query and symbol
felixfbecker Jan 16, 2017
91580a6
Add ServerCapability
felixfbecker Jan 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions extension-workspace-references.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,26 @@ interface SymbolLocationInformation {
}
```
* error: code and message set in case an exception happens during the definition request.


### Extended Workspace Symbol Request

The `workspace/symbol` request takes an optional parameter `symbol` that allows you to query by known properties about the symbol.
The string `query` parameter becomes optional.
Copy link

Choose a reason for hiding this comment

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

@felixfbecker think it might be good to give examples of the different use cases of query and symbol?

Copy link
Author

Choose a reason for hiding this comment

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

Will add an examples section.


```typescript
/**
* The parameters of a Workspace Symbol Request.
*/
interface WorkspaceSymbolParams {
/**
* A query string
*/
query?: string;

/**
* Known properties about the symbol.
*/
symbol?: Partial<SymbolDescriptor>;
Copy link
Member

Choose a reason for hiding this comment

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

Since we must modify the type of query to be optional with this extensive anyway, what is the reason to have the fields be separate?

i.e. why not just have query be either a string or a Partial<SymbolDescriptor> ?

Copy link
Author

Choose a reason for hiding this comment

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

I thought about that too, but there are languages that are strictly typed and cannot have it either be a string or an object. LSP will avoid these union types in the future, see microsoft#39. There isn't really a disadvantage to it, but it increases the likelihood of the proposal getting accepted later.

Copy link
Author

Choose a reason for hiding this comment

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

query symbol
user input in UI used programmatically
matches as fuzzily as possible matches as exact as possible
returns as many results as possible returns as few results as possible

Copy link
Member

@emidoots emidoots Jan 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense, and sounds good. In that case:

  1. How about naming it querySymbol instead?
  2. Can we clarify that only one of query or querySymbol should be present? If not, what is the behavior of a LS when both are specified?

Copy link
Author

Choose a reason for hiding this comment

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

  1. I like to keep it short and precise - if any, I would prefer symbolQuery, and that is kind of confusing since the whole thing you are doing is a symbol query. I thought about naming it symbolDescriptor, but it is not a full SymbolDescriptor, and I would like to change it to SymbolInformation soon.
  2. Yes, this should be specified. The semantics should be an AND, because it could be useful to query with a fuzzy query but restrict it to a specific SymbolKind or package.

}
```