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

Extract commands from new protocol inside a dedicated library #1758

Merged
merged 2 commits into from
May 13, 2024

Conversation

xvw
Copy link
Collaborator

@xvw xvw commented May 5, 2024

This PR is based on voodoos/502 (#1757)

In order to easily enable tunneling, via a custom query from ocaml-lsp, a large part of the argument processing logic (argv) can be used. This branch separates the command invocation logic (and the dispatching of CLI arguments to useful merlin-lib functionality) into a dedicated library, which can be used from within ocaml-lsp.

@xvw xvw force-pushed the tunneling-merlin-lsp-p1-split-protocol branch from 4500bed to ed5b519 Compare May 6, 2024 16:02
@xvw xvw changed the title Draft: Split protocol and argv parsing into a dedicated library Draft: Extract commands from new protocol inside a dedicated library May 6, 2024
@xvw xvw changed the title Draft: Extract commands from new protocol inside a dedicated library Extract commands from new protocol inside a dedicated library May 6, 2024
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 @xvw !

It's nice to realize that Query_json actually doesn't depends on Yojson :-)

Did you also check that other important parsing function such as the one parsing the flags to build the configuration are already exposed by merlin lib ?

src/commands/dune Outdated Show resolved Hide resolved
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.

These changes LGTM.

Were you able to confirm that this is actually enough to implement the "tunneling" on the LSP side ?

@xvw
Copy link
Collaborator Author

xvw commented May 13, 2024

Thanks @xvw !

It's nice to realize that Query_json actually doesn't depends on Yojson :-)

Did you also check that other important parsing function such as the one parsing the flags to build the configuration are already exposed by merlin lib ?

Yes. I am integrating test into my ocaml-lsp branch. I'll do the PR today.

@voodoos
Copy link
Collaborator

voodoos commented May 13, 2024

@xvw I merged the upgrade of master to 5.2, could you rebase that PR ? Thank you !

@xvw xvw force-pushed the tunneling-merlin-lsp-p1-split-protocol branch from 24e9794 to 089ee97 Compare May 13, 2024 15:24
@xvw
Copy link
Collaborator Author

xvw commented May 13, 2024

Done!

xvw added 2 commits May 13, 2024 17:39
Moves the commands in the new protocol to a dedicated library (used by
the binary) which can be used by `ocaml-lsp`.
@xvw xvw force-pushed the tunneling-merlin-lsp-p1-split-protocol branch from 089ee97 to 46e4faf Compare May 13, 2024 15:39
@voodoos voodoos merged commit 25bb400 into ocaml:master May 13, 2024
5 of 9 checks passed
voodoos added a commit to voodoos/opam-repository that referenced this pull request May 17, 2024
CHANGES:

Fri May 17 19:59:42 CET 2024

  + merlin binary
    - Support for OCaml 5.2 (ocaml/merlin#1757)
    - destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
    - Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
      fixes ocaml/merlin#1661)
    - Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
    - Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
    - Improve cursor position detection in longidents (ocaml/merlin#1756)
    - Addition of a `merlin-lib.commands` library which disassociates the
      execution of commands from the `new_protocol`, from the binary, allowing
      it to be invoked from other projects (ocaml/merlin#1758)
    - New occurrences backend: Don't index occurrences when `merlin.hide`
      attribute is present. (ocaml/merlin#1768)
    - Use the new `uid_to_decl` table in 5.2's cmt files to get documentation.
      (ocaml/merlin#1773)
voodoos added a commit to voodoos/merlin that referenced this pull request Jun 10, 2024
…lit-protocol

Extract commands from new protocol inside a dedicated library
voodoos added a commit to voodoos/merlin that referenced this pull request Jun 10, 2024
…lit-protocol

Extract commands from new protocol inside a dedicated library
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 10, 2024
CHANGES:

Mon Jun 10 17:35:42 CEST 2024

  + merlin binary
    - Addition of a `merlin-lib.commands` library which disassociates the
      execution of commands from the `new_protocol`, from the binary, allowing
      it to be invoked from other projects (ocaml/merlin#1758)
    - `merlin-lib.commands`: Add a `find_command_opt` alternative to
      `find_command` that does not raise (ocaml/merlin#1778)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 10, 2024
CHANGES:

Mon Jun 10 17:35:42 CEST 2024

  + merlin binary
    - Addition of a `merlin-lib.commands` library which disassociates the
      execution of commands from the `new_protocol`, from the binary, allowing
      it to be invoked from other projects (ocaml/merlin#1758)
    - `merlin-lib.commands`: Add a `find_command_opt` alternative to
      `find_command` that does not raise (ocaml/merlin#1778)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 10, 2024
CHANGES:

Mon Jun 10 17:35:42 CEST 2024

  + merlin binary
    - Addition of a `merlin-lib.commands` library which disassociates the
      execution of commands from the `new_protocol`, from the binary, allowing
      it to be invoked from other projects (ocaml/merlin#1758)
    - `merlin-lib.commands`: Add a `find_command_opt` alternative to
      `find_command` that does not raise (ocaml/merlin#1778)
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

Mon Jun 10 17:35:42 CEST 2024

  + merlin binary
    - Addition of a `merlin-lib.commands` library which disassociates the
      execution of commands from the `new_protocol`, from the binary, allowing
      it to be invoked from other projects (ocaml/merlin#1758)
    - `merlin-lib.commands`: Add a `find_command_opt` alternative to
      `find_command` that does not raise (ocaml/merlin#1778)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 4.16-414
Status: 4.16-501
Development

Successfully merging this pull request may close these issues.

2 participants