Make code-lens for toplevel let binding configurable
#1567
Conversation
Tim-ats-d
force-pushed
the
configurable-internal-code-lens
branch
from
008ce82 to
a59d7d1
Compare
Tim-ats-d
force-pushed
the
configurable-internal-code-lens
branch
from
a59d7d1 to
cfe0f89
Compare
xvw
left a comment
xvw
left a comment
There was a problem hiding this comment.
Can you add some e2e-new test to validate the behaviour? Thanks!
Tim-ats-d
force-pushed
the
configurable-internal-code-lens
branch
from
2b870ff to
0d47e2a
Compare
xvw
left a comment
xvw
left a comment
There was a problem hiding this comment.
Last change :) Thanks @Tim-ats-d !
There was a problem hiding this comment.
Sorry to be that guy, but I think the choice of option is unnecessary restrictive here. What if one day we add lenses for other things and want to have it finely configurable ? Then with an option named "only_toplevel" we are stuck.
Let's make it additive instead (there might be a better name for it): "for_nested_bindings" with a default to false.
Does that make sense ?
So you mean the name for the option is not really well-chosen? |
|
(We could also over engineer it with a list: "only: [toplevel_bindings; nested_bindin]" but maybe that's not worth the complexity) |
The semantic of the option itself is too restrictive. |
Okay I think I see what you mean. I've changed the semantic of the option in my last commit. What do you think? |
…rmat-mlx * 'master' of github.com:/ocaml/ocaml-lsp: Rename parameter used to configure nested bindings. (ocaml#1568) Upgrade to ocamlformat 0.28.1 (ocaml#1569) Improve precision of duration field in `view-metrics` (ocaml#1565) Make `code-lens` for toplevel let binding configurable (ocaml#1567) Fix 5.4 CI Enable support for OCaml 5.4 and prepare release 1.24.0 (ocaml#1559) Prepare release 1.23.1 (ocaml#1558)
3 participants
This PR adds an
only_toplevelfield to CodeLens settings, allowing CodeLens to display only for toplevelletbindings.Solving #1561