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

lsp-copilot: Replace LSP server with the official GitHub one #4710

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

suzuki
Copy link
Contributor

@suzuki suzuki commented Feb 13, 2025

About

Resolve #4708

Replace the LSP server used by the lsp-copilot client with the official GitHub one.
https://www.npmjs.com/package/@github/copilot-language-server

@github-actions github-actions bot added the client One or more of lsp-mode language clients label Feb 13, 2025
@suzuki suzuki marked this pull request as ready for review February 13, 2025 09:20
@@ -211,6 +211,7 @@ automatically, browse to %s." user-code verification-uri))
("$/progress" (lambda (&rest args) (lsp-message "$/progress with %S" args)))
("featureFlagsNotification" #'ignore)
("statusNotification" #'ignore)
("didChangeStatus" #'ignore)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4708 (comment)

I'm seeing a bunch of Warning (lsp-mode): Unknown notification: didChangeStatus messages. Not too familiar w/ LSP, but do we need to set up some additional handling for these notifications?

https://github.com/orgs/github/packages/npm/package/copilot-language-server

The status is sent to the client as a didChangeStatus notification. Typically this would be conveyed to the user in a status bar icon or other UI affordance.

I think it would be fine to simply ignore it for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

didChangeStatus does not seem to be a LSP (the protocol) standard notification, it's copilot-specific.

From https://www.npmjs.com/package/@github/copilot-language-server :

Status Notification

The status is sent to the client as a didChangeStatus notification. Typically this would be conveyed to the user in a status bar icon or other UI affordance.

Notification includes the following fields:

message - a string describing the status (can be empty)
kind - status of the language server:
    'Normal' - When everything is working normally.
    'Error' - When not authorized and authenticated.
    'Warning' - When there is a temporary issue, such as a failed HTTP request.
    'Inactive' - When the current file is ignored due to file size or content exclusions.

Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to handle this due to the Inactive kind -- it'd be good to know that a given file is not getting completions because of token limits ...

Copy link

@dylnclrk dylnclrk Feb 13, 2025

Choose a reason for hiding this comment

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

New to emacs dev, but would you just want to print messages to the lsp log buffer when you get the inactive messages?

Copy link
Contributor

Choose a reason for hiding this comment

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

mostly yes -- use lsp--info, --error, --warn -- that'll make to the lsp-mode log buffer and to the minibuffer, so the user has some visual feedback

@kassick
Copy link
Contributor

kassick commented Feb 13, 2025

@jcs090218 jcs090218 merged commit ad38dab into emacs-lsp:master Feb 14, 2025
10 of 13 checks passed
@jcs090218
Copy link
Member

LGTM, thank you!

@suzuki suzuki deleted the replace-lsp-copilot-server branch February 14, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copilot-node-server is no longer maintained
4 participants