Add quirk support for initialize parameters

[anticomputer]

Certain language servers require non-standard parameters to be passed as part of their initialization. Currently eglot does not support providing additional base parameters outside of the
initializationOptions and capabilities.

Adding a generic function that defaults to an empty list allows users to provide custom parameters to deal with such quirks.

An example is the CodeQL language server, which expects the workspaceFolders parameter to set.

With these changes in place you can support such this language server quirk with something like:

(defclass eglot-codeql (eglot-lsp-server) ()
  :documentation "A custom class for CodeQL's langserver.")

(cl-defmethod eglot-client-initialize-params ((server eglot-codeql))
  "Add :workspaceFolders parameter to EGLOT initialization for SERVER of type EGLOT-CODEQL."
  (let ((root (car (project-roots (eglot--project server)))))
    `(:workspaceFolders [(:name "workspace" :uri ,(eglot--path-to-uri root))])))

(add-to-list 'eglot-server-programs
             `((ql-mode) .
               (eglot-codeql . ("<path to codeql executable>"
                                "execute" "language-server"
                                "--search-path" "./:<path to codeql repo>"
                                "--check-errors" "ON_CHANGE"
                                "-q" "--log-to-stderr"))))

[joaotavora]

For consistency with the current code and with https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize, shouldn't the new entry point eglot-workspace-folders?

Else, it could indeed be called eglot-initialize-params and function more or less as you pretend. Then its default specification should call the existing eglot-initialization-options in its (t) primary method. And we'd have to specify what overriding semantics to use (should the new method be allowed override things like processId and rootPath)?

So I'm leaning to the first option and providing that spec in the default spec, if the server has the workspaceFolders capability. Also eglot-path-to-uri should be made public.

[joaotavora]

I also forgot to say "thanks" for the contribution :-) Also hailing @antifuchs here who was proposing something similar I think.

[anticomputer]

For consistency with the current code and with https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize, shouldn't the new entry point eglot-workspace-folders?

Else, it could indeed be called eglot-initialize-params and function more or less as you pretend. Then its default specification should call the existing eglot-initialization-options in its (t) primary method. And we'd have to specify what overriding semantics to use (should the new method be allowed override things like processId and rootPath)?

So I'm leaning to the first option and providing that spec in the default spec, if the server has the workspaceFolders capability. Also eglot-path-to-uri should be made public.

Ah yeah if it's captured as part of the spec then it's less of a quirk and agree it should just be specific to workspaceFolders then. I'll iterate a bit to make it more clean/direct.

[anticomputer]

Ok, simplified the PR as per the spec, adding workspace folder configurations now looks like:

  (defclass eglot-codeql (eglot-lsp-server) ()
    :documentation "A custom class for CodeQL's langserver.")

  (cl-defmethod eglot-workspace-folders ((server eglot-codeql))
    "Add :workspaceFolders parameter to EGLOT initialization for SERVER of type EGLOT-CODEQL."
    (let ((root (car (project-roots (eglot--project server)))))
      `[(:name "workspace" :uri ,(eglot--path-to-uri root))]))

  (add-to-list 'eglot-server-programs
             `((ql-mode) .
               (eglot-codeql . ("<path to codeql executable>"
                                "execute" "language-server"
                                "--search-path" "./:<path to codeql repo>"
                                "--check-errors" "ON_CHANGE"
                                "-q" "--log-to-stderr"))))

[nemethf]

It's great that @anticomputer started to work on this. Unfortunatelly, I don't really like the approach in the PR. Not that my opinion should matter much. Nevertheless, I think Eglot should have a proper support for workspaceFolders first, then we can start to think about how Eglot can cope with a misbehaving server like CodeQL.

Do you know why CodeQL requires the client to send the actual workspaceFolders early in the initialize request, and why CodeQL cannot send a workspace/workspaceFolders request to the client as the LSP specification dictates?

About the usage example: project-roots is deprecated. (But the multiple directories in project-roots would nicely map to multiple workspaceFolders, so it is surprising that the example only uses the first one.)

[joaotavora]

@nemethf i agree. And your opinion matters :)

I think it's good to grow this PR to properly support workspace folders, not just a quirk.

And your idea for the default mapping with project-roots makes sense. I fact project.el should be the true source for wsfolders not Eglot, who is just a middleman.

[nemethf]

And your idea for the default mapping with project-roots makes sense.

More like project-root + project-external-roots, but yes. project-roots does not exists anymore.

I fact project.el should be the true source for wsfolders not Eglot, who is just a middleman.

I agree with this as well. I wonder though what Eglot should do with eglot-extend-to-xref in this context. I think extend-to-xref exists because people had difficulties to configure project-external-roots correctly, but it is debatable whether a file accessed via extend-to-xref belongs to the workspace. Hm. Ignoring extend-to-xref would lead to a simpler implementation, which later could be revisited if necessary.

[joaotavora]

Hm. Ignoring extend-to-xref would lead to a simpler implementation, which later could be revisited if necessary.

Yep!

[anticomputer]

It's great that @anticomputer started to work on this. Unfortunatelly, I don't really like the approach in the PR. Not that my opinion should matter much. Nevertheless, I think Eglot should have a proper support for workspaceFolders first, then we can start to think about how Eglot can cope with a misbehaving server like CodeQL.

Do you know why CodeQL requires the client to send the actual workspaceFolders early in the initialize request, and why CodeQL cannot send a workspace/workspaceFolders request to the client as the LSP specification dictates?

About the usage example: project-roots is deprecated. (But the multiple directories in project-roots would nicely map to multiple workspaceFolders, so it is surprising that the example only uses the first one.)

To put my PR in context: I'm not deeply familiar with eglot codebase or LSP spec, mostly it's the result of hacking around (what seemed to me) a quirk to get CodeQL langserver to work :) This is similar to how the nvim client for CodeQL bootstraps the workspaceFolders, in that they just set workspaceFolders[0] to rootUri.

Happy to dig in a little deeper and bring this up to standard though if y'all can come to a consensus.

This specific behavior was extracted from the official CodeQL vscode extension, which sends the workspaceFolders as part of the initialize params as well. The langserver does not function without it, but I can see if it can be implemented as a regular change notification from the client.

[joaotavora]

Sure, @anticomputer your PR is very good. I'd be most happy if it can somehow be made forward compatible to @nemethf 's idea of "deeper" wsfolders support. I think it can. I'll review this properly next week.

[joaotavora]

There's also the question of copyright assignment for emacs contributions. You should get that started: please write to [email protected] and request a copyright assignment form.

[nemethf]

This specific behavior was extracted from the official CodeQL vscode extension, which sends the workspaceFolders as part of the initialize params as well.

Can you provide a link to this part of the source code of the official extension? Because looking at the communication log in github/vscode-codeql#629 I think this is not necessary.

[anticomputer]

This specific behavior was extracted from the official CodeQL vscode extension, which sends the workspaceFolders as part of the initialize params as well.

Can you provide a link to this part of the source code of the official extension? Because looking at the communication log in github/vscode-codeql#629 I think this is not necessary.

Hrmm Im afk currently, but that's interesting. I'll see if we can do the initialization with just folder change events.

To be clear, the folks you're referencing in that thread are coworkers of mine :P Another colleague maintains his personal nvim client and they initialize the workspaceFolders[0] with rootUri as part of the initialization params, based on a trace they got from the vscode extension.

This is separate from the additional events that add/remove database archives. The langserver does not work with no workspaceFolders initialization, but it might be that the project root can just be added by a normal folder change event.

Either way, it seems initializing workspaceFolders as part of the server initialize parameters should be supported as a first step since that's documented in the spec @joaotavora linked to earlier: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize

Subsequent folder add/deletes are then part of a broader "support workspace folders completely" discussion I reckon.

Anyhoo, I'll dig a little deeper and maybe ping some folks on the extension team when I'm back at work.

[anticomputer]

To wit, maybe a better name would be eglot-workspace-folders-initialize for this one?

I'll note that the spec says rootPath and rootUri are deprecated in favor of workspaceFolders so perhaps it might even make sense to always just set workspaceFolders[0] off of the default-directory uri in the initialize params, but that might be a little too aggressive.

[nemethf]

Either way, it seems initializing workspaceFolders as part of the server initialize parameters should be supported as a first step since that's documented in the spec @joaotavora linked to earlier: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize

You're right. I read this part of the specification many times, and yet still managed to misunderstand it. Sorry.

So duplicating the current rootUri as a single element of the workspaceFolders in the initialize request is in line with the specification and would make CodeQL happy as well. Moreover, Eglot can have a basic workspaceFolders support if it additionally fills this list with the content of project-external-roots. Seems pretty straightforward.

[anticomputer]

So duplicating the current rootUri as a single element of the workspaceFolders in the initialize request is in line with the specification and would make CodeQL happy as well. Moreover, Eglot can have a basic workspaceFolders support if it additionally fills this list with the content of project-external-roots. Seems pretty straightforward.

I've added a commit that takes this first step of setting workspaceFolders[0] to rootUri in the initialize request, and verified it makes CodeQL lang server happy by default without requiring any specialization from the user.

[anticomputer]

There is some useful documentation for further implementation available here as well: https://github.com/microsoft/vscode/wiki/Adopting-Multi-Root-Workspace-APIs#language-client--language-server

[anticomputer]

Ah cool, #893 looks more thorough than what I'm proposing here, will close this PR, thanks!