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

feat(ocamllsp): support Pexp_ifthenelse folding ranges #1031

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

tatchi
Copy link
Collaborator

@tatchi tatchi commented Feb 15, 2023

Support folding ranges for ifthenelse expressions.

Before:

Screen.Recording.2023-02-15.at.06.50.12.mov

After:

Screen.Recording.2023-02-15.at.06.50.32.mov

As you can see in the video above, we can collapse the entire if expression but not the else. This is because we don't have the start location of the else keyword. We only have the location of the whole expression inside the else. We can add this location to the ranges, but it doesn't appear next to the else.

Screen.Recording.2023-02-15.at.06.52.38.mov

Also, it makes it impossible to collapse only the first expression inside the else.

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

Update CHANGES please

@rgrinberg
Copy link
Member

@voodoos do you want to rebase and merge this?

@coveralls
Copy link

coveralls commented Jan 29, 2024

Pull Request Test Coverage Report for Build 4088

  • -4 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.003%) to 20.381%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/folding_range.ml 0 4 0.0%
Totals Coverage Status
Change from base Build 4082: -0.003%
Covered Lines: 4851
Relevant Lines: 23801

💛 - Coveralls

@voodoos
Copy link
Collaborator

voodoos commented Jan 29, 2024

@rgrinberg are you familiar with this CI failure ?

--- a/_build/default/ocaml-lsp-server/test/e2e-new/workspace_change_config.ml
+++ b/_build/default/ocaml-lsp-server/test/e2e-new/workspace_change_config.ml.corrected
@@ -68,7 +68,35 @@ let string = "Hello"
   in
 
   test source req;
-  [%expect {| CodeLens found: 0 |}]
+  [%expect {|
+    (* CR expect_test_collector: This test expectation appears to contain a backtrace.
+       This is strongly discouraged as backtraces are fragile.
+       Please change this test to not include a backtrace. *)
+
+    CodeLens found: 0
+    detached: /-----------------------------------------------------------------------
+    | Internal error: Uncaught exception.
+    | ("Lev_fiber.Io: already closed",
+    | { source =
+    |     "Raised by primitive operation at Lev_fiber.Io.callstack.(fun) in file \"submodules/lev/lev-fiber/src/lev_fiber.ml\", line 590, characters 31-58\n\
+    |      Called from Lev_fiber.Io.create in file \"submodules/lev/lev-fiber/src/lev_fiber.ml\", line 743, characters 17-29\n\
+    |      Called from Lev_fiber.Io.Lazy_fiber.force.(fun) in file \"submodules/lev/lev-fiber/src/lev_fiber.ml\", line 1047, characters 23-27\n\
+    |      Called from Fiber__Scheduler.exec in file \"fiber/src/scheduler.ml\", line 73, characters 8-11\n\
+    |      Called from Fiber__Scheduler.start in file \"fiber/src/scheduler.ml\", line 216, characters 2-42\n\
+    |      Called from Fiber.run.(fun) in file \"fiber/src/fiber.ml\" (inlined), line 17, characters 28-47\n\
+    |      Called from Lev_fiber.run in file \"submodules/lev/lev-fiber/src/lev_fiber.ml\", line 1264, characters 10-59\n\
+    |      Called from Ocaml_lsp_server.run in file \"ocaml-lsp-server/src/ocaml_lsp_server.ml\", line 936, characters 2-151\n\
+    |      Called from Stdune__Exn_with_backtrace.try_with in file \"otherlibs/stdune/src/exn_with_backtrace.ml\", line 9, characters 8-12\n\
+    |      Called from Dune__exe__Main in file \"ocaml-lsp-server/bin/main.ml\", line 41, characters 6-106\n\
+    |      "
+    | })
+    | Raised at Stdune__Code_error.raise in file "otherlibs/stdune/src/code_error.ml", line 10, characters 30-62
+    | Called from Lev_fiber.Io.with_ in file "submodules/lev/lev-fiber/src/lev_fiber.ml", line 983, characters 10-62
+    | Called from Fiber__Scheduler.exec in file "fiber/src/scheduler.ml", line 73, characters 8-11
+    | Re-raised at Stdune__Exn.raise_with_backtrace in file "otherlibs/stdune/src/exn.ml" (inlined), line 38, characters 27-56
+    | Called from Stdune__Exn_with_backtrace.reraise in file "otherlibs/stdune/src/exn_with_backtrace.ml", line 20, characters 33-71
+    | Called from Fiber__Scheduler.exec in file "fiber/src/scheduler.ml", line 73, characters 8-11
+    \----------------------------------------------------------------------- |}]

@rgrinberg
Copy link
Member

Yes, I've seen them before. Our tests aren't very careful about making sure that we close the client only after we receive or reject incoming packets from the server. I intend to fix this eventually, but it's only an issue that's observable in the tests (nobody spins up lsp servers to serve a single request) so priority is low

@voodoos voodoos merged commit a74fb4a into ocaml:master Jan 30, 2024
9 checks passed
PizieDust added a commit to PizieDust/ocaml-lsp that referenced this pull request Mar 8, 2024
feat(ocamllsp): support Pexp_ifthenelse folding ranges (ocaml#1031)

* test(ocamllsp): add test for Pexp_ifthenelse folding ranges
* feat(ocamllsp): support Pexp_ifthenelse folding ranges

remove configuration via environment variables

update docs

syntax highlighting for code block

make syntax highlighter required
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jul 5, 2024
CHANGES:

## Features

- Introduce a configuration option to control dune diagnostics. The option is
  called `duneDiganostics` and it may be set to `{ enable: false }` to disable
  diagnostics. (ocaml/ocaml-lsp#1221)

- Support folding of `ifthenelse` expressions (ocaml/ocaml-lsp#1031)

- Improve hover behavior (ocaml/ocaml-lsp#1245)

  Hovers are no longer displaye on useless parsetree nodes such as keywords,
  comments, etc.

  Multiline hovers are now filtered away.

  Display expanded ppx's in the hover window.

- Improve document symbols (ocaml/ocaml-lsp#1247)

  Use the parse tree instead of the typed tree. This means that document
  symbols will work even if the source code doesn't type check.

  Include symbols at arbitrary depth.

  Differentiate functions / types / variants / etc.

  This now includes PPXs like `let%expect_test` or `let%bench` in the outline.

- Introduce a `destruct-line` code action. This is an improved version of the
  old `destruct` code action. (ocaml/ocaml-lsp#1283)

- Improve signature inference to only include types for elements that were
  absent from the signature. Previously, all signature items would always be
  inserted. (ocaml/ocaml-lsp#1289)

- Add an `update-signature` code action to update the types of elements that
  were already present in the signature (ocaml/ocaml-lsp#1289)

- Add custom
  [`ocamllsp/merlinCallCompatible`](https://github.com/ocaml/ocaml-lsp/blob/e165f6a3962c356adc7364b9ca71788e93489dd0/ocaml-lsp-server/docs/ocamllsp/merlinCallCompatible-spec.md)
  request (ocaml/ocaml-lsp#1265)

- Add custom [`ocamllsp/typeEnclosing`](https://github.com/ocaml/ocaml-lsp/blob/109801e56f2060caf4487427bede28b824f4f1fe/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md) request (ocaml/ocaml-lsp#1304)

## Fixes

- Detect document kind by looking at merlin's `suffixes` config.

  This enables more lsp features for non-.ml/.mli files. Though it still
  depends on merlin's support. (ocaml/ocaml-lsp#1237)

- Correctly accept the `--clientProcessId` flag. (ocaml/ocaml-lsp#1242)

- Disable automatic completion and signature help inside comments (ocaml/ocaml-lsp#1246)

- Includes a new optional/configurable option to toggle syntax documentation. If
  toggled on, allows display of syntax documentation on hover tooltips. Can be
  controlled via environment variables and by GUI for VS code. (ocaml/ocaml-lsp#1218)

- For completions on labels that the LSP gets from merlin, take into account
  whether the prefix being completed starts with `~` or `?`. Change the label
  completions that start with `?` to start with `~` when the prefix being
  completed starts with `~`. (ocaml/ocaml-lsp#1277)

- Fix document syncing (ocaml/ocaml-lsp#1278, ocaml/ocaml-lsp#1280, fixes ocaml/ocaml-lsp#1207)

- Stop generating inlay hints on generated code (ocaml/ocaml-lsp#1290)

- Fix parenthesizing of function types in `SignatureHelp` (ocaml/ocaml-lsp#1296)

- Fix syntax documentation rendering (ocaml/ocaml-lsp#1318)
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.

None yet

4 participants