Skip to content

Conversation

@xvw
Copy link
Collaborator

@xvw xvw commented Apr 10, 2025

At the moment, there is only one request to return the list of typed holes, which means that a lot of client-side logic has to be implemented. For example, in ocaml-eglot (see):

;; Holes

;; TODO: It would probably be possible to improve the query to embed
;; more logic at the `merlin-lib` level rather than calculating hole
;; logic at the editor level.

(defun ocaml-eglot--first-hole-aux (holes pos comparison)
  "Return the first hole of the list HOLES since a POS using COMPARISON."
  (when (or holes (not (equal [] holes)))
    (let* ((hd (car holes))
           (tl (cdr holes))
           (h-start (cl-getf hd :start))
           (cmp (ocaml-eglot-util--compare-position h-start pos)))
      (if (funcall comparison cmp 0) hd
        (ocaml-eglot--first-hole-aux tl pos comparison)))))

(defun ocaml-eglot--first-hole-at (holes pos comparison)
  "Return the first hole of the list HOLES since a POS using COMPARISON.
If there is no available holes, it returns the first one of HOLES."
  (let ((hole (ocaml-eglot--first-hole-aux holes pos comparison)))
    (if hole hole (car holes))))

(defun ocaml-eglot--get-first-hole-in (start end)
  "Return the first hole in a given range denoted by START and END."
  (let* ((holes (ocaml-eglot-req--holes))
         (hole (ocaml-eglot--first-hole-at holes start '>=)))
    (when hole
      (let ((hole-start (cl-getf hole :start))
            (hole-end (cl-getf hole :end)))
        (when (and (>= (ocaml-eglot-util--compare-position hole-start start) 0)
                   (<= (ocaml-eglot-util--compare-position hole-end end) 0))
          hole)))))

(defun ocaml-eglot--first-hole-in (start end)
  "Jump to the first hole in a given range denoted by START and END."
  (when-let* ((hole (ocaml-eglot--get-first-hole-in start end))
              (hole-start (cl-getf hole :start)))
    (ocaml-eglot-util--jump-to hole-start)))

(defun ocaml-eglot-hole-prev ()
  "Jump to the previous hole."
  (interactive)
  (ocaml-eglot-req--server-capable-or-lose :experimental :ocamllsp :handleTypedHoles)
  (let* ((current-pos (eglot--pos-to-lsp-position))
         (holes (reverse (ocaml-eglot-req--holes)))
         (hole (ocaml-eglot--first-hole-at holes current-pos '<)))
    (when hole (ocaml-eglot-util--jump-to-range hole))))

(defun ocaml-eglot-hole-next ()
  "Jump to the next hole."
  (interactive)
  (ocaml-eglot-req--server-capable-or-lose :experimental :ocamllsp :handleTypedHoles)
  (let* ((current-pos (eglot--pos-to-lsp-position))
         (holes (ocaml-eglot-req--holes))
         (hole (ocaml-eglot--first-hole-at holes current-pos '>)))
    (when hole (ocaml-eglot-util--jump-to-range hole))))

This patch allows the next/previous hole to be related to a given piece of contextual information. It is possible to include a range to return a hole only in a given range.

(cc @awilliambauer)

@xvw xvw requested a review from voodoos April 10, 2025 15:21
@xvw xvw changed the title More control on typed holes (for navigation) DRAFT: More control on typed holes (for navigation) Apr 10, 2025
@coveralls
Copy link

coveralls commented Apr 10, 2025

Pull Request Test Coverage Report for Build 4833

Details

  • 51 of 60 (85.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 23.15%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/typed_hole.ml 24 25 96.0%
ocaml-lsp-server/src/custom_requests/req_typed_holes.ml 0 2 0.0%
ocaml-lsp-server/src/custom_requests/req_jump_to_typed_hole.ml 27 33 81.82%
Totals Coverage Status
Change from base Build 4826: 0.2%
Covered Lines: 5934
Relevant Lines: 25633

💛 - Coveralls

@xvw xvw changed the title DRAFT: More control on typed holes (for navigation) More control on typed holes (for navigation) Apr 10, 2025
@xvw xvw force-pushed the more-control-on-typed-holes branch from bb7b065 to b2debd6 Compare April 10, 2025 20:19
@xvw xvw force-pushed the more-control-on-typed-holes branch from c37704e to dbbf627 Compare April 11, 2025 19:10
@xvw xvw changed the title More control on typed holes (for navigation) DRAFT: More control on typed holes (for navigation) Apr 11, 2025
@xvw xvw changed the title DRAFT: More control on typed holes (for navigation) More control on typed holes (for navigation) Apr 11, 2025
@xvw xvw requested a review from voodoos April 11, 2025 19:40
@xvw xvw force-pushed the more-control-on-typed-holes branch from fd6ece9 to 3cea330 Compare April 11, 2025 19:41
@xvw xvw force-pushed the more-control-on-typed-holes branch from 3cea330 to 12c06c5 Compare April 11, 2025 19:53
Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

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

Thanks, I agree that moving this logic to the server is useful: these position based tricks are frequent footguns so it's good that client implementers can just trust the server instead of re-implementing it.

@xvw xvw force-pushed the more-control-on-typed-holes branch from c939746 to 87f31ec Compare April 11, 2025 19:59
@xvw xvw merged commit 53bbb83 into ocaml:master Apr 11, 2025
6 checks passed
@xvw xvw deleted the more-control-on-typed-holes branch April 11, 2025 20:19
voodoos pushed a commit to voodoos/ocaml-lsp that referenced this pull request Jun 6, 2025
* Introduce a new custom request `ocamllsp/jumpTypedHole`

* Test the custom request

* Add CHANGE entry

* Add more tests related to range

* Add specification

* Simplify request description

* Refactor typed-holes usage in a dedicated module

* Rename request name

* Rephrase documentation

* Rename `name` to `pipeline_name`
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 23, 2025
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)

- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)

- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)

- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)

- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)

- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)

- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)

- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)

- Add a new server option `standardHover`, that can be used by clients to
  disable the default hover provider.  When `standardHover = false`
  `textDocument/hover` requests always returns with empty result. (ocaml/ocaml-lsp#1416)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 23, 2025
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)

- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)

- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)

- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)

- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)

- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)

- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)

- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)

- Add a new server option `standardHover`, that can be used by clients to
  disable the default hover provider.  When `standardHover = false`
  `textDocument/hover` requests always returns with empty result. (ocaml/ocaml-lsp#1416)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 24, 2025
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)
- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)
- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)
- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)
- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)
- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)
- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)
- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 24, 2025
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)
- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)
- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)
- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)
- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)
- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)
- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)
- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)
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.

3 participants