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

support settings through didChangeConfiguration notification #1103

Merged
merged 6 commits into from
May 31, 2023

Conversation

aspeddro
Copy link
Contributor

@aspeddro aspeddro commented May 24, 2023

This PR add support to enable/disable settings through didChangeConfiguration notification.

Settings

  • CodeLens
  • Extended Hover

Example using Neovim

require('lspconfig').ocamllsp.setup {
  cmd = {'ocamllsp'},
  on_attach = on_attach,
  settings = {
    extendedHover = { 
      enable = true 
    },
    codelens = {
      enable = false
    }
  },
  capabilities = capabilities,
}

Close #1102

@coveralls
Copy link

coveralls commented May 24, 2023

Pull Request Test Coverage Report for Build 3748

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 2 of 104 (1.92%) changed or added relevant lines in 3 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 16.818%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/ocaml_lsp_server.ml 1 6 16.67%
ocaml-lsp-server/src/configuration.ml 1 14 7.14%
ocaml-lsp-server/src/config_data.ml 0 84 0.0%
Files with Coverage Reduction New Missed Lines %
ocaml-lsp-server/src/configuration.ml 4 18.18%
Totals Coverage Status
Change from base Build 3733: -0.07%
Covered Lines: 3934
Relevant Lines: 23391

💛 - Coveralls

@rgrinberg rgrinberg requested a review from 3Rafal May 24, 2023 07:43
@3Rafal
Copy link
Collaborator

3Rafal commented May 24, 2023

Please add a changelog note here:
https://github.com/ocaml/ocaml-lsp/blob/master/CHANGES.md

@3Rafal
Copy link
Collaborator

3Rafal commented May 24, 2023

We have tests for:
hover
hover extended
code lens

Can we add tests that change default configuration and check if it is applied?

Copy link
Collaborator

@3Rafal 3Rafal left a comment

Choose a reason for hiding this comment

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

Thanks @aspeddro for this PR. It looks good, but I think it would be nice to add suggested improvements. Feel free to ask me any questions in case something is not clear. :)

@aspeddro
Copy link
Contributor Author

Related to this PR:

We can remove the ocamllsp/hoverExtended request introduced in #561. This is now supported by most clients and is within specification.

@rgrinberg rgrinberg requested a review from ddickstein May 30, 2023 12:54
@rgrinberg rgrinberg added this to the 1.16.0 milestone May 30, 2023
@rgrinberg rgrinberg merged commit 0860f0c into ocaml:master May 31, 2023
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 18, 2023
CHANGES:

## Fixes

- Disable code lens by default. The support can be re-enabled by explicitly
  setting it in the configuration. (ocaml/ocaml-lsp#1134)

- Fix initilization of `ocamlformat-rpc` in some edge cases when ocamlformat is
  initialized concurrently (ocaml/ocaml-lsp#1132)

- Kill unnecessary `$ dune ocaml-merlin` with SIGTERM rather than SIGKILL
  (ocaml/ocaml-lsp#1124)

- Refactor comment parsing to use `odoc-parser` and `cmarkit` instead of
  `octavius` and `omd` (ocaml/ocaml-lsp#1088)

  This allows users who migrated to omd 2.X to install ocaml-lsp-server in the
  same opam switch.

  We also slightly improved markdown generation support and fixed a couple in
  the generation of inline heading and module types.

- Allow opening documents that were already open. This is a workaround for
  neovim's lsp client (ocaml/ocaml-lsp#1067)

- Disable type annotation for functions (ocaml/ocaml-lsp#1054)

- Respect codeActionLiteralSupport capability (ocaml/ocaml-lsp#1046)

- Fix a document syncing issue when utf-16 is the position encoding (ocaml/ocaml-lsp#1004)

- Disable "Type-annotate" action for code that is already annotated.
  ([ocaml/ocaml-lsp#1037](ocaml/ocaml-lsp#1037)), fixes
  [ocaml/ocaml-lsp#1036](ocaml/ocaml-lsp#1036)

- Fix semantic highlighting of long identifiers when using preprocessors
  ([ocaml/ocaml-lsp#1049](ocaml/ocaml-lsp#1049), fixes
  [ocaml/ocaml-lsp#1034](ocaml/ocaml-lsp#1034))

- Fix the type of DocumentSelector in cram document registration (ocaml/ocaml-lsp#1068)

- Accept the `--clientProcessId` command line argument. (ocaml/ocaml-lsp#1074)

- Accept `--port` as a synonym for `--socket`. (ocaml/ocaml-lsp#1075)

- Fix connecting to dune rpc on Windows. (ocaml/ocaml-lsp#1080)

## Features

- Add "Remove type annotation" code action. (ocaml/ocaml-lsp#1039)

- Support settings through `didChangeConfiguration` notification (ocaml/ocaml-lsp#1103)

- Add "Extract local" and "Extract function" code actions. (ocaml/ocaml-lsp#870)

- Depend directly on `merlin-lib` 4.9 (ocaml/ocaml-lsp#1070)
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.

Add support to disable settings through didChangeConfiguration
4 participants