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

Add __id to LSP completions #4839

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tobias-tengler
Copy link
Contributor

Two of our developers reached out, because they weren't sure if they're using __id in the right places, since the LSP wouldn't suggest it.

I've extended the LSP completions to include the __id field to clear up those confusions.

To my knowledge the __id field should be a valid selection on any complex type. If there are special cases, please let me know and I'll update the implementation for the suggestion accordingly.

Copy link
Contributor

@captbaritone captbaritone left a comment

Choose a reason for hiding this comment

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

Good catch. Overall looks good. One question before I import.

fn resolve_completion_items_typename(type_: Type, schema: &SDLSchema) -> Vec<CompletionItem> {
fn resolve_special_completion_items(type_: Type, schema: &SDLSchema) -> Vec<CompletionItem> {
let __id_item = CompletionItem {
label: "__id".to_owned(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be exposed by the SDLSchema? A little nervous having this fieldname and type hard coded in multiple modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants