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

ocaml-lsp: init at 2020-07-07 #92675

Closed
wants to merge 1 commit into from
Closed

Conversation

marsam
Copy link
Contributor

@marsam marsam commented Jul 8, 2020

Motivation for this change

Add https://github.com/ocaml/ocaml-lsp

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@vbgl
Copy link
Contributor

vbgl commented Jul 8, 2020

Merged ppx-yojson-conv-lib in master as fa535b5

Do you know how to test the ocaml-lsp stuff?

@marsam
Copy link
Contributor Author

marsam commented Jul 9, 2020

I think you need to configure your editor to work with a Language Server client.
If you use Emacs, I think lsp-mode supports ocaml-lsp out-of-the-box. It should provide completions/syntax checking/docs/etc.

image

Copy link
Contributor

@vbgl vbgl left a comment

Choose a reason for hiding this comment

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

Still not managed to get this working. But I could crash the server using emacs & eglot:

[stderr] /-----------------------------------------------------------------------
[stderr] | Internal error: Uncaught exception.
[stderr] | ("invalid language id", { language_id = "tuareg" })
[stderr] | Raised at file "vendor/stdune/code_error.ml", line 9, characters 30-62
[stderr] | Called from file "ocaml-lsp-server/src/ocaml_lsp_server.ml", line 66, characters 12-31
[stderr] | Called from file "lsp/src/scheduler.ml", line 368, characters 29-51
[stderr] | Called from file "vendor/fiber/fiber.ml", line 165, characters 8-13
[stderr] -----------------------------------------------------------------------

@@ -0,0 +1,33 @@
{ lib, fetchFromGitHub, ocamlPackages }:

with ocamlPackages; buildDunePackage rec {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you use buildDunePackage (as you seem to customize the build and install phases nonetheless)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no special reason, at first I tried to use buildDunePackage with the default phases, but it didn't work.
Should I change it to stdenv.mkDerivation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use what you think is more appropriate.

@@ -0,0 +1,33 @@
{ lib, fetchFromGitHub, ocamlPackages }:

with ocamlPackages; buildDunePackage rec {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the version of OCaml used for compiling this LSP server matter? Should it be the same as the version used for development?

For merlin, e.g., it does matter and this is why the merlin package is inside ocamlPackages rather than taking it as argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't tested thoroughly with different OCaml versions, but IIUC it may depend on the Ocaml version

@marsam
Copy link
Contributor Author

marsam commented Jul 9, 2020

But I could crash the server using emacs & eglot:

ocaml-lsp uses textDocumentItem'slanguageId to identify the OCaml syntax to be used.
Currently eglot builds languageId from major-mode, and doesn't offer a way to customize it; so you need patch it for tuareg-mode:

--- i/eglot.el
+++ w/eglot.el
@@ -1648,9 +1648,11 @@ THINGS are either registrations or unregisterations (sic)."
   (append
    (eglot--VersionedTextDocumentIdentifier)
    (list :languageId
-         (if (string-match "\\(.*\\)-mode" (symbol-name major-mode))
-             (match-string 1 (symbol-name major-mode))
-           "unknown")
+         (if (eq major-mode 'tuareg-mode)
+             "ocaml"
+           (if (string-match "\\(.*\\)-mode" (symbol-name major-mode))
+               (match-string 1 (symbol-name major-mode))
+             "unknown"))
          :text
          (eglot--widening
           (buffer-substring-no-properties (point-min) (point-max))))))

I didn't have the time to submit a proper patch to Eglot

@vbgl
Copy link
Contributor

vbgl commented Aug 28, 2020

Version 1.0.0 is out: https://github.com/ocaml/ocaml-lsp/releases/tag/1.0.0

@vbgl
Copy link
Contributor

vbgl commented Nov 8, 2020

Closed by #103133

@vbgl vbgl closed this Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants