Support verbosity=smart by ulugbekna · Pull Request #1374 · ocaml/merlin

ulugbekna · 2021-07-27T15:05:46Z

What the PR does

Implements support for verbosity=smart.

Note: I tried to keep changes for adding verbosity=smart minimal.

Includes a couple of clean ups, I can remove them from this PR if necessary.

Also includes a fix for: The format for MERLIN_LOG env var is different in two places in merlin new_merlin.ml and ocamlmerlin_server.ml -- MERLIN_LOG= filename{,section}* or filename

Note: I noticed that a part of functions in Type_utils use the global mutable verbosity as a way to avoid passing verbosity as an argument along the call hierarchy, another half - passed verbosity around. I could perhaps fix this if there is clear preference. It also seems slightly more involved change than this one.

Reviewing

It is best to go commit-by-commit.

In the end, it looks quite good on the ocaml-lsp's side:

ulugbekna · 2021-07-27T15:06:56Z

src/analysis/completion.ml

          if not exact then raise Exit;
-          let verbosity = !Type_utils.verbosity in
+          let verbosity = 
+            Mconfig.Verbosity.to_int !Type_utils.verbosity ~for_smart:1


In most cases, I tried to keep in mind ocaml-lsp's use case for verbosity=smart, but I'm not sure what to put here.

ulugbekna · 2021-11-17T13:28:43Z

Can I jump on the current reviews train? :D

voodoos · 2021-11-22T10:32:13Z

Can I jump on the current reviews train? :D

Sorry, it will have to be after the imminent release.

I am curious: is that already in-use in OCaml-lsp ?

ulugbekna · 2021-11-22T11:15:15Z

I am curious: is that already in-use in OCaml-lsp ?

Unfortunately not, but it should be

cc @rgrinberg

rgrinberg · 2021-11-22T13:44:42Z

We can experiment with this patch in lsp first if you'd like, but it needs to be a lot smaller. For starters, get rid of all nonessential changes to ease the maintenance.

voodoos · 2021-11-22T15:01:24Z

Our priority right now is the release but I can have a look shortly after if the goal is to have it upstreamed to not rely on another ocaml-lsp side's patch.

ulugbekna · 2021-11-22T15:47:49Z

Indeed that’s the goal.

…

On Mon, 22 Nov 2021 at 16:01, Ulysse ***@***.***> wrote: Our priority right now is the release but I can have a look shortly after if the goal is to have it upstreamed to not rely on another ocaml-lsp side. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1374 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4YR67EBXTVWZFEV2AQ6BTUNJLM7ANCNFSM5BCN65QA> .

voodoos · 2021-11-30T17:38:36Z

@ulugbekna could you move the clean-ups/refactoring parts of the PR to another one as you suggested ?

ulugbekna · 2021-11-30T18:09:30Z

I hope to do so this week :-)

…

On Tue, 30 Nov 2021 at 18:38, Ulysse ***@***.***> wrote: @ulugbekna <https://github.com/ulugbekna> could you move the clean-ups/refactoring parts of the PR to another one as you suggested ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1374 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4YR65C5JWZECTOXMPLRULUOUD2NANCNFSM5BCN65QA> .

voodoos

Thanks a lot @ulugbekna the changes mostly look good to me :-)

src/kernel/mconfig.ml

src/analysis/type_utils.ml

src/kernel/mconfig.ml

src/analysis/type_utils.ml

ulugbekna · 2022-11-14T10:45:38Z

Thanks for a prompt review! Addressed all comments

voodoos · 2022-11-15T09:51:37Z

We also need to update the protocol documentation:
https://github.com/ocaml/merlin/blob/master/doc/dev/PROTOCOL.md#type-enclosing--position-position---expression-string----cursor-int----index-int-

ulugbekna · 2022-11-16T12:16:40Z

but that document doesn't mention -verbosity at all?

voodoos · 2022-11-16T12:55:20Z

but that document doesn't mention -verbosity at all?

🧐 looks like a great opportunity to do it 😅

ulugbekna · 2022-11-16T12:58:12Z

In another PR perhaps?

…which more clearly states what the function does

Co-authored-by: Ulysse <5031221+voodoos@users.noreply.github.com>

@mheiber

CHANGES: Thu Nov 24 13:31:42 CEST 2022 + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Do not change temporarily Merlin's cwd when starting a PPX (ocaml/merlin#1521, fixes ocaml/merlin#1420) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) - Add a new `verbosity=smart` mode for type enclosing that only expand modules' types (ocaml/merlin#1374, @ulugbekna) - Improve locate for labels' declarations in the current buffer. (ocaml/merlin#1505, fixes ocaml/merlin#1524) - Fix locate on module without implementation (ocaml/merlin#1522, fixes ocaml/merlin#1519) - Allow program name customization when merlin is used as a library. (ocaml/merlin#1532) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) - emacs: update CI for newer releases and fix some warnings (ocaml/merlin#1454, @mattiase) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comment documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512) - Add tests for interactions between locate and record labels (ocaml/merlin#1505) - Add test showing an issue with locate and implicit transitive deps

@mheiber

CHANGES: + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Do not change temporarily Merlin's cwd when starting a PPX (ocaml/merlin#1521, fixes ocaml/merlin#1420) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) - Add a new `verbosity=smart` mode for type enclosing that only expand modules' types (ocaml/merlin#1374, @ulugbekna) - Improve locate for labels' declarations in the current buffer. (ocaml/merlin#1505, fixes ocaml/merlin#1524) - Fix locate on module without implementation (ocaml/merlin#1522, fixes ocaml/merlin#1519) - Allow program name customization when merlin is used as a library. (ocaml/merlin#1532) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) - emacs: update CI for newer releases and fix some warnings (ocaml/merlin#1454, @mattiase) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comment documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512) - Add tests for interactions between locate and record labels (ocaml/merlin#1505) - Add test showing an issue with locate and implicit transitive deps [new release] merlin (4.7-413) CHANGES: + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comments documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512) [new release] merlin (4.7-412) CHANGES: + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comment documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512)

@mheiber

CHANGES: Thu Nov 24 17:49:42 CEST 2022 + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Do not change temporarily Merlin's cwd when starting a PPX (ocaml/merlin#1521, fixes ocaml/merlin#1420) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) - Add a new `verbosity=smart` mode for type enclosing that only expand modules' types (ocaml/merlin#1374, @ulugbekna) - Improve locate for labels' declarations in the current buffer. (ocaml/merlin#1505, fixes ocaml/merlin#1524) - Fix locate on module without implementation (ocaml/merlin#1522, fixes ocaml/merlin#1519) - Allow program name customization when merlin is used as a library. (ocaml/merlin#1532) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) - emacs: update CI for newer releases and fix some warnings (ocaml/merlin#1454, @mattiase) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comment documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512) - Add tests for interactions between locate and record labels (ocaml/merlin#1505) - Add test showing an issue with locate and implicit transitive deps

@mheiber

CHANGES: + merlin binary - Replace custom "holes" AST nodes by extensions. This restores binary compatibility and fixes issues with PPXs when using typed-holes. (ocaml/merlin#1503) - Do not change temporarily Merlin's cwd when starting a PPX (ocaml/merlin#1521, fixes ocaml/merlin#1420) - Fix a parsing issue when declaring the `(??)` custom prefix operator. (ocaml/merlin#1507, fixes ocaml/merlin#1506) - Fix variant constructors' comments grouping (ocaml/merlin#1516, @mheiber, fixes ocaml/merlin#1513) - Filter-out duplicates from the `enclosing` command result (ocaml/merlin#1512) - Add a new `verbosity=smart` mode for type enclosing that only expand modules' types (ocaml/merlin#1374, @ulugbekna) - Improve locate for labels' declarations in the current buffer. (ocaml/merlin#1505, fixes ocaml/merlin#1524) - Fix locate on module without implementation (ocaml/merlin#1522, fixes ocaml/merlin#1519) - Allow program name customization when merlin is used as a library. (ocaml/merlin#1532) + editor modes - vim: load the plugin when necessary if it wasn't loaded before (ocaml/merlin#1511) - emacs: update CI for newer releases and fix some warnings (ocaml/merlin#1454, @mattiase) + test suite - Add tests for constructors' documentation (ocaml/merlin#1511) - Add test cases for label comment documentation (ocaml/merlin#1526, @mheiber) - Add a test for the `enclosing` command (ocaml/merlin#1512) - Add tests for interactions between locate and record labels (ocaml/merlin#1505) - Add test showing an issue with locate and implicit transitive deps

ulugbekna commented Jul 27, 2021

View reviewed changes

ulugbekna force-pushed the verbosity-smart branch from 550dd21 to 8a0f7e6 Compare August 2, 2021 10:30

ulugbekna force-pushed the verbosity-smart branch from 8a0f7e6 to 13273d5 Compare September 6, 2021 11:25

ulugbekna force-pushed the verbosity-smart branch from 13273d5 to 74ad49d Compare November 17, 2021 13:27

voodoos added this to the 4.5 / 3.8 milestone Nov 22, 2021

voodoos self-requested a review November 22, 2021 15:09

ulugbekna mentioned this pull request Nov 28, 2021

Add a new custom command ocamllsp/hoverExtended with support for variable verbosity ocaml/ocaml-lsp#561

Merged

voodoos modified the milestones: 4.5 / 3.8, Bridge the gap with OCaml-LSP Dec 14, 2021

voodoos mentioned this pull request Nov 7, 2022

Reproduce issue #1335 #1336

Closed

ulugbekna force-pushed the verbosity-smart branch from 74ad49d to a5ed76e Compare November 14, 2022 08:18

voodoos added this to the 4.7 / 3.9 milestone Nov 14, 2022

voodoos requested changes Nov 14, 2022

View reviewed changes

ulugbekna force-pushed the verbosity-smart branch from e5bef4a to 1a1f0eb Compare November 14, 2022 10:44

voodoos force-pushed the verbosity-smart branch from 1a1f0eb to 87847ce Compare November 17, 2022 14:00

rename Type_utils.Printtyp.(select_verbose -> select_by_verbosity) …

d47b627

…which more clearly states what the function does

ulugbekna and others added 8 commits November 17, 2022 15:18

add verbosity=smart support

029ae22

fix param spec

5cd3fcf

Co-authored-by: Ulysse <5031221+voodoos@users.noreply.github.com>

don't mention ocamllsp

91ce7e2

Co-authored-by: Ulysse <5031221+voodoos@users.noreply.github.com>

formatting

67dbb47

Co-authored-by: Ulysse <5031221+voodoos@users.noreply.github.com>

improve '-verbosity' description

609bbd2

use !verbosity when expanding type

387a544

Add a changelog entry

7b19a30

Add protocol documentation

fb6048e

voodoos force-pushed the verbosity-smart branch from 87847ce to fb6048e Compare November 17, 2022 14:35

voodoos merged commit a6a53d6 into ocaml:master Nov 17, 2022

voodoos mentioned this pull request Nov 24, 2022

[new release] merlin (4.7-414, 4.7-413, 4.7-412) and merlin-lib (4.7-414) ocaml/opam-repository#22555

Merged

voodoos mentioned this pull request Nov 24, 2022

[new release] merlin and merlin-lib (4.7-500) ocaml/opam-repository#22561

Merged

Conversation

ulugbekna commented Jul 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulugbekna Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

ulugbekna commented Nov 17, 2021

Uh oh!

voodoos commented Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulugbekna commented Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Nov 22, 2021

Uh oh!

voodoos commented Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ulugbekna commented Nov 22, 2021 via email

Uh oh!

voodoos commented Nov 30, 2021

Uh oh!

ulugbekna commented Nov 30, 2021 via email

Uh oh!

voodoos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ulugbekna commented Nov 14, 2022

Uh oh!

voodoos commented Nov 15, 2022

Uh oh!

ulugbekna commented Nov 16, 2022

Uh oh!

voodoos commented Nov 16, 2022

Uh oh!

ulugbekna commented Nov 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ulugbekna commented Jul 27, 2021 •

edited

Loading

voodoos commented Nov 22, 2021 •

edited

Loading

ulugbekna commented Nov 22, 2021 •

edited

Loading

voodoos commented Nov 22, 2021 •

edited

Loading