feature: disable code lens by default

[rgrinberg]

This feature seems to annoy quite a few users so we disable it by
default. It can be enabled through configuration.

Signed-off-by: Rudi Grinberg [email protected]

[aspeddro]

Update the config.md

ocaml-lsp/ocaml-lsp-server/docs/ocamllsp/config.md

Lines 14 to 20 in 16c2d41

	/**
	* Enable/Disable CodeLens
	* @default true
	* @since 1.16
	*/
	codelens: { enable : boolean }
	}

[coveralls]

Pull Request Test Coverage Report for Build 3852

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.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.004%) to 17.353%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
ocaml-lsp-server/src/ocaml_lsp_server.ml	1	2	50.0%

Totals
Change from base Build 3848:	0.004%
Covered Lines:	4073
Relevant Lines:	23471

---

💛 - Coveralls

[Khady]

FWIW, this is a terrible user experience for anyone not very comfortable with their editor and the ocaml environment. This is definitely something that is expected to work by default. And the fix is completely non obvious.

[beajeanm]

This feature seems to annoy quite a few users

It's the first time I've heard of this being an issue, what problem were they seeing with code lense enabled by default?

so we disable it by default

Default is king, this mean most user will assume ocaml tooling doesn't support code lens because they'll won't be aware of the config switch, or how to configure their particular flavor of editor/lsp-plugin to have that switch forwarded to ocaml-lsp.

[3Rafal]

I have approved this PR because it was technically correct, but considering above arguments I'd say that we should bring back default code lenses.

[rgrinberg]

There's just as many users who complained about this feature to me personally as there are people in here that are in favor of it. There can only be one default so unfortunately, we will need to upset one group.

I suggest that we play it conservatively with controversial features and disable them by default. If we didn't make it easy enough to re-enable this feature, that's a very valid complaint.

As a side note, @jfeser is adding inlay hints which are going to be strictly superior to this feature anyway and we'll definitely enable that by default.

[tmattio]

There's just as many users who complained about this feature to me personally as there are people in here that are in favor of it. There can only be one default so unfortunately, we will need to upset one group.

It would be nice to move these complains to the issue tracker, so there's an opportunity to have a discussion. What is the reason they are annoyed by the feature?

[rgrinberg]

The complaint is fairly simple: they take up a lot of vertical real estate and provide little value. The problem is compounded when other plugins want to provide code lenses. They aren't actionable, so they don't even fulfill their original spec.

@ddickstein @hackwaly @aspeddro can expand as well. These are just the names I remember who complained about this feature.

[tmattio]

Thanks!

I suggest that we play it conservatively with controversial features and disable them by default. If we didn't make it easy enough to re-enable this feature, that's a very valid complaint.

My opinion on this is that when choosing which defaults to use, we should prioritise new users since users who are advanced enough to know what they want are able to configure their editors to match that. That's also the opinion of @Khady above IIUC.

• Do new OCaml users find the code lens valuable? (I have a strong intuition that it is valuable to understand code, especially as a beginner. I've personally found it useful on many occasions)
• Are the people you tagged happy to have a configuration option to disable code lense instead of changing the default? (implemented in Support lsp server settings ocamllabs/vscode-ocaml-platform#1157 for vscode)

[hackwaly]

I would accept that given an config option to disable it rather than make disable as default. Since newcomes may not know if there is codelens for types

[rgrinberg]

Okay, you guys can bring it back if you want. Heads up, it's still going away in favor of inlay hints as those are designed for type annotations unlike the code lens.

[tmattio]

Will the inlay hints provide type annotations for top-level values/functions? It does seem to be the case in the screenshot on #1159. If it does, then the information is duplicated and I would agree that code lens can be replaced with inlay hints, otherwise it seems useful to have both

[rgrinberg]

Yes, they will work for top-level functions and values. Although they are not that intrusive so we can enable them everywhere.

[tmattio]

Ok, if the information is duplicated, then I'm siding with you and I don't think it's useful to bring the code lens back (as the default). Would be nice to introduce the inlay hints at the same time as the removal of code lens (as the default) so we don't have a release with neither.

[beajeanm]

Indeed it looks like inlay hints will be the way to good, thanks for the information.

[dyedgreen]

Some context on why I raised this with @rgrinberg originally:

• It already duplicates information (even before inlay type annotations are introduced) since the same information is available in the provided document symbols
• Inlay type annotations solve this problem better (since they look like the source could would look rather than having special styling, eg in VSCode the code lens actions are not in a monospaced font)
• The type code lens was also confusing, because it’s not actionable (eg we have some code lenses for running inline tests, which are effectively inline buttons in the code)

[Khady]

• being able to see types directly isn't the same as having to rely on more complicated mechanisms which require an action from the developer
• Indeed inlay look great, but so far the feature hasn't been developed so this is not a valid replacement
• It is not clickable but it is definitely actionable, for example I can write code based on the types I see

Sure I'm happy to replace the codelens with inlay when the latter lands. But in the meantime a useful feature was (relatively silently) dropped with no replacement and no quickfix but forking ocamllsp. Maybe for such breaking changes if there isn't a good support in all major editors ocamllsp should provide a workaround. It could be a CLI argument to the ocamllsp binary or an environment variable for example.

we have some code lenses for running inline tests, which are effectively inline buttons in the code

the name of the config option to disable the codelens is probably too generic then? As one might want to disable the type codelens but keep the other kind of codelens?

[cemerick]

Big 👍 on this being a poor change, absent an obvious alternative. I was sure either the vscode extension or ocamllsp were borked (as I've always used the presence of the codelenses as the primary indication that the extension and lsp server were talking properly), and was about to write up something equivalent to #1164 before I thankfully saw it waiting for me.

(I'm also far from sold on "inlays" as being obviously superior based on what I see in #1159, but perhaps that's a separate topic.)

Could someone please ELI5 how to restore the codelens functionality? Getting hints from trawling through various issues and commits, I've tried these configuration, with no results:

"ocaml.codelens.enable": true,
"[ocaml]": {
    "editor.codeLens": true
}

[aspeddro]

@cemerick, the config is "ocaml.server.codelens": true

[beajeanm]

For anyone coming here looking for the solution for neovim, if you are using lsp-config, you will need to add to your ocamllsp setup something like:

require('lspconfig').ocamllsp.setup {
  settings = {
    codelens = { enable = true },
 },
}

[EduardoRFS]

So, this was disabled by default, there is a bug which makes it not enabled in VSCode for a while and inlay hints hasn't been merged yet. Can we enable it by default again?