Skip to content

Comments

feat: add setting to enable LSP#191

Open
amtoney524 wants to merge 9 commits intols-clientfrom
add-enable-lsp-setting
Open

feat: add setting to enable LSP#191
amtoney524 wants to merge 9 commits intols-clientfrom
add-enable-lsp-setting

Conversation

@amtoney524
Copy link
Contributor

@amtoney524 amtoney524 commented Nov 6, 2025

This PR defines a setting to enable LSP.

Extension settings:
Screenshot 2025-11-06 at 6 35 53 PM

Filtered settings:
Screenshot 2025-11-06 at 6 36 03 PM

package.json Outdated
"configuration": {
"title": "Colab",
"properties": {
"colab.codeDiagnostics.LSP": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as a starting point, "Code Diagnostics" came from Colab Web's Editor settings

Image

Copy link
Member

Choose a reason for hiding this comment

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

I would suspect the vast majority of users don't know what "language server protocol (LSP)" is. I'd like to name the setting (and have a description as) something which generically represents the functionality.

E.g. Code diagnostics - Enable code diagnostics powered by a connected Colab server.

@EvanWiederspan may have some suggestions too.

Should look at the verbiage the Python and Pylance extensions use. Or perhaps other language server extensions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice suggestion, updated to that as a starting point! I think you're right about not using the term "LSP" - I can only find one setting that references that

here's an example from the pylance extension where type checking modes are exposed
Screenshot 2025-11-06 at 6 44 19 PM

@amtoney524 amtoney524 requested a review from kevineger November 6, 2025 18:13
@amtoney524 amtoney524 marked this pull request as ready for review November 6, 2025 18:14
Copy link
Member

@kevineger kevineger left a comment

Choose a reason for hiding this comment

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

Let's wait to merge this until the setting does something (is wired up).

@amtoney524 amtoney524 force-pushed the add-enable-lsp-setting branch from ac0b994 to ca93fa2 Compare November 13, 2025 00:33
@amtoney524 amtoney524 changed the base branch from main to ls-client November 13, 2025 00:33
@kevineger kevineger force-pushed the ls-client branch 2 times, most recently from 99b4b8a to 5435139 Compare December 3, 2025 20:18
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