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.

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

Jump to bottom

RFC: Implement workspace/xfiles and textDocument/xcontent #211

Closed
wants to merge 1 commit into from
Closed

RFC: Implement workspace/xfiles and textDocument/xcontent #211

wants to merge 1 commit into from

Conversation

nemethf
Copy link
Collaborator

@nemethf nemethf commented Jan 14, 2019

I know @joaotavora doesn't really like non-standard protocol extensions, but one way of accelerating standardization is to have many independent implementation of a proposal. The files extensions document list many use-cases, but I'd like to add one more: supporting tramp-files would be straightforward.

I've tried to implement these extensions and it seems to work with the Dockerfile server from here, but the containerized lua server throws seemingly independent errors. The php language server supports these extensions out of the box, and it seems to do something, but I get errors like:

internal (id:116) ERROR Mon Jan 14 15:02:19 2019:
(:message "error ignored, status set (Method LanguageServer\\Server\\TextDocument::documentHighlight() does not exist)" :id 116 :error -32601)

Anyway, I do not have a deep understanding of eglot or LSP, so I'm not confident with this PR, but I'm curious what you think about it.

Thanks.

@nemethf
Implement workspace/xfiles and textDocument/xcontent
4621c3f 
See https://github.com/sourcegraph/language-server-protocol/blob/master/extension-files.md

* eglot.el (eglot-client-capabilities): Extend with xfilesProvider
and xcontentProvider.
(eglot-xfiles-visible-regexp, eglot-xfiles-hidden-regexp): New
defcustoms.
(eglot--path-to-TextDocumentIdentifier, eglot--xfiles-visible-p)
(eglot-handle-request workspace/xfiles)
(eglot-handle-request textDocument/xcontent): New functions.
@joaotavora
Copy link
Owner

I know @joaotavora doesn't really like non-standard protocol extensions,

I'd have no problem if you make a eglot-x.el library somewhere that accumulates non-standard protocol extensions. From reading the code it seems that the current eglot API is sufficient (if it isn't, do let me know). Once these non-standard things start making it into the standard, they can be migrated to Eglot.

@nemethf
Copy link
Collaborator Author

nemethf commented Jan 14, 2019

By "somewhere" do you mean somewhere other than this repository? :)
OK. I close this PR then.

@nemethf nemethf closed this Jan 14, 2019
@joaotavora
Copy link
Owner

By "somewhere" do you mean somewhere other than this repository? :)

Hmmm, for now yes, but it could be eglot-x.el besides eglot later on, and it might be in emacs core even. Do you have signed FSF papers, btw?

@nemethf
Copy link
Collaborator Author

nemethf commented Jan 14, 2019

I haven't signed the papers, but that wouldn't be a problem. I just haven't contributed anything yet.

@joaotavora
Copy link
Owner

I haven't signed the papers, but that wouldn't be a problem. I just haven't contributed anything yet.

Ask on emacs-devel@gnu.org that someone mail you the papers.

@nemethf
Copy link
Collaborator Author

nemethf commented Jan 23, 2019

I've made a second iteration of the idea and I've also finished the paperwork. (It was surprisingly easy.)
Even if you don't want to support the files extension in eglot, can you, please, take a look at the implementation? I'd appreciate any feedback. Thanks.
https://github.com/nemethf/eglot/blob/xfiles-v2/eglot-x.el

@joaotavora
Copy link
Owner

Of course I can! Thanks

@nemethf nemethf mentioned this pull request Jan 23, 2019
Open
@nemethf nemethf mentioned this pull request Oct 11, 2019
Merged
nemethf added a commit to nemethf/eglot-x that referenced this pull request Nov 13, 2019
@nemethf
Init the repo
eefd3fc 
Start a new repository since @joaotavora didn't want to support
non-standard features in eglot.
joaotavora/eglot#211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@nemethf @joaotavora