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.

Sign up for GitHub

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

Jump to bottom

New ocaml-merlin subcommand acting as a configuration server for next Merlin #3395

Merged
merged 20 commits into from
May 15, 2020
Merged

New ocaml-merlin subcommand acting as a configuration server for next Merlin #3395

merged 20 commits into from
May 15, 2020

Conversation

voodoos
Copy link
Collaborator

@voodoos voodoos commented Apr 20, 2020 •
edited
Loading

Twin PR: ocaml/merlin#1123

This PR adds a new ocaml-merlin subcommand to Dune.

Behavior

When started (usually by merlin) this program will wait for input on stdin, When it receives a request for a files' configuration it will read the content of the corresponding merlin configuration file and print it to stdout. The empty list is returned if no configuration is found.

Each message is formed with one canonical s-expression. The test contain an example of a dialogue between Merlin and this configuration server, the most important part being:

  $ dune ocaml-merlin

  input:   
    (4:File9:/project_path/main.ml)

  output: 
    (
     (17:EXCLUDE_QUERY_DIR)
     (1:B65:/path/to/some/lib)
     (3:FLG105:-w @[email protected]@30..39@[email protected]@[email protected] etc)
    )

Future versions of Merlin will check for the presence of a dune or dune-project file in the sources and decide if they should use the standard configuration reader or this new Dune subcommand.

In the future this new behaviour will allow for a finer grained merlin configuration: actual .merlin files work at the folder level while dune will be able to answer precisely for each file.

Breaking changes

This PR disables the promotion of .merlin files to the sources. Thus it is not compatible with actual version of Merlin.

Todo

  • Correct merlin related tests
  • Add tests for the new configuration server

@voodoos voodoos mentioned this pull request Apr 21, 2020
Merged
1 task
@voodoos voodoos changed the title New ocaml-merlin subcommand acting as a configuration server for next Merlin [WIP] ocaml-merlin subcommand acting as a configuration server for next Merlin Apr 22, 2020
@voodoos voodoos force-pushed the ocaml-merlin branch 5 times, most recently from 259a2ad to a687879 Compare May 13, 2020 13:31
@voodoos
Copy link
Collaborator Author

voodoos commented May 13, 2020

This PR is now ready for review.

However it should not be merged before the corresponding changes in Merlin are merged and released. (Before merging the incompatibility with previous versions of Merlin should also be added to the package dependency.)

@voodoos voodoos marked this pull request as ready for review May 13, 2020 14:31
@voodoos voodoos changed the title [WIP] ocaml-merlin subcommand acting as a configuration server for next Merlin ocaml-merlin subcommand acting as a configuration server for next Merlin May 13, 2020
@voodoos voodoos changed the title ocaml-merlin subcommand acting as a configuration server for next Merlin New ocaml-merlin subcommand acting as a configuration server for next Merlin May 13, 2020
ghost
ghost reviewed May 14, 2020
View reviewed changes
src/dune/merlin_server.ml Outdated Show resolved Hide resolved
ghost
ghost reviewed May 14, 2020
View reviewed changes
src/dune/merlin_server.ml Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented May 14, 2020

@voodoos the PR looks ready to me!

I would just rename Dot to Dot_merlin, and you also need to revert the changes related to the renaming .merlin -> .merlin-conf. You should feel free to merge once this is done.

voodoos added 9 commits May 15, 2020 12:57
@voodoos
Add a simple ocaml-merlin subcommand
c595025 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Implement new merlin_server feedback loop
68956b5 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Disable generation of .merlin files in sources (Break tests)
764da5e 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Rename internal merlin file
d4472ef 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
disable-promotion test does not make sense anymore (.merlin-conf ne…
57c51ae 
…ver promoted)

Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Refactor
33aef07 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Use correct workspace
33054aa 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Compat with 4.07
a30f6ea 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Reformat
8125c07 
Signed-off-by: Ulysse Gérard <[email protected]>
voodoos added 10 commits May 15, 2020 12:57
@voodoos
Halt on EOF and non-csexp input
b435cbb 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
More doc
ed38b7d 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add test for new merlin config server
4fcbaf2 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Remove useless line break in output
338ecd3 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Correct comment
ea4e7ed 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Reformat
c50836c 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Rename Dot to Dot_melrin
179d594 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Revert "Disable promotion of .merlin files"
67edcf0 
This reverts commit 868d09c.

Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Revert renaming of melrin files
a235a50 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Revert "disable-promotion test suppression"
7d10413 
This reverts commit e1f9724.

Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos
Add changelog entry
02bee73 
Signed-off-by: Ulysse Gérard <[email protected]>
@voodoos voodoos merged commit e5bec1a into ocaml:master May 15, 2020
@rgrinberg rgrinberg mentioned this pull request Jun 6, 2020
Merged
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 6, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
f80ea5a 
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
411eb5d 
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
feeccf5 
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
@hongchangwu hongchangwu mentioned this pull request Sep 24, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aalekseyev aalekseyev Awaiting requested review from aalekseyev

@emillon emillon Awaiting requested review from emillon

@nojb nojb Awaiting requested review from nojb

@rgrinberg rgrinberg Awaiting requested review from rgrinberg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@voodoos