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

Merlin config handling #697

Merged
merged 1 commit into from
May 12, 2022
Merged

Conversation

rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented May 9, 2022

The first few commits are just general cleanup and refactoring. The remaining commits, introduce reference counting for processes ran by lsp to get the merlin config.

@anuragsoni once this this ready, it should fix #694

@rgrinberg rgrinberg force-pushed the ps/rr/refactor__merlin_config_handling branch 3 times, most recently from b26845e to 25808fe Compare May 9, 2022 21:19
@rgrinberg rgrinberg requested review from ulugbekna May 9, 2022 23:06
@rgrinberg rgrinberg force-pushed the ps/rr/refactor__merlin_config_handling branch from 25808fe to 515f956 Compare May 9, 2022 23:07
@rgrinberg rgrinberg changed the title merlin config handling refactor Merlin config handling May 9, 2022
@rgrinberg rgrinberg added this to the 1.11.5 milestone May 10, 2022
@ulugbekna
Copy link
Collaborator

I will get to review this tomorrow. Sorry for the wait

Copy link
Collaborator

@ulugbekna ulugbekna left a comment

Choose a reason for hiding this comment

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

changes look correct to me

@rgrinberg rgrinberg force-pushed the ps/rr/refactor__merlin_config_handling branch from 515f956 to e273ea0 Compare May 12, 2022 19:08
everything that handles the configuration data type should live in a
submodule

ps-id: 04F23C4C-6FC9-432F-BEE5-EB98B9FC99A6
@rgrinberg rgrinberg force-pushed the ps/rr/refactor__merlin_config_handling branch from e273ea0 to 7c346fc Compare May 12, 2022 19:09
@rgrinberg rgrinberg merged commit 603327c into master May 12, 2022
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request May 12, 2022
CHANGES:

- Fix process termination. Once the lsp server is stepped, the process will
  gracefully terminate (ocaml/ocaml-lsp#697, fixes ocaml/ocaml-lsp#694)

- Forward stderr from dune's merlin configuration to the lsp server's stderr
  (ocaml/ocaml-lsp#697)
@anuragsoni
Copy link

Thank you! I can confirm that pinning to ocaml-lsp on this commit fixes the #694

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.

ocamllsp leaves an idle process running after editor is closed
3 participants