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

[TypeScript] Support placeholder in LSP type output #3846

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

Thom1729
Copy link
Collaborator

LSP output, such as is displayed in tooltips, truncated long types with syntax like the following:

type T = Foo | ... 100 more ... | Bar;

This is not valid TypeScript syntax, but LSP highlights tooltips using the core TS syntax, and this causes highlighting to break. (The TS type expression syntax is deliberately rigid to prevent false positives.)

This PR adds support for the ... 100 more ... bit to the TS syntax definition. Ordinarily, I'd hesitate to put something that's not actually TS syntax in the core definition, but in this case I think it makes sense — given the tight coupling between the language and the language server, I'm not concerned about this conflicting with other syntax extensions.

I used a comment scope, though not for any particular reasons. The regexp seems to work in practice. I have inquired on the official TypeScript Discord about whether there is any documentation for this, or whether the placeholder is localizable, and so on, but I am not holding my breath for a helpful response.

Copy link
Collaborator

@michaelblyons michaelblyons left a comment

Choose a reason for hiding this comment

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

Seems fine to me. I am not opinionated about the scope for this.

@deathaxe
Copy link
Collaborator

As long as it doesn't break valid TypeScript, it's ok, IMHO.

@Thom1729 Thom1729 merged commit 16a2650 into sublimehq:master Sep 23, 2023
@Thom1729 Thom1729 deleted the typescript-lsp-ellipses branch September 23, 2023 19:07
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.

3 participants