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

look for .merlin in _build #712

Closed
wants to merge 1 commit into from
Closed

Conversation

vogler
Copy link

@vogler vogler commented Aug 29, 2017

Closes issue #686
After looking for dir/.merlin this also tries dir/_build/.merlin before going up the path.

@let-def
Copy link
Contributor

let-def commented Sep 11, 2017

Thanks. I am not opposed to the idea, bug different approaches have been suggested, especially for interop with jbuilder.
@vogler which build system are you using?

@vogler
Copy link
Author

vogler commented Sep 11, 2017

jbuilder 😄 See jbuilder/#207.
Just seemed like the easiest fix for now.
Which other approaches? Using jbuild directly would of course be even better.

@rdavison
Copy link

@let-def just curious, what other approaches are you referring to?

@let-def
Copy link
Contributor

let-def commented Oct 2, 2017

@rdavison I was thinking of supporting jbuild directly or provide an interface for plugging external build system plugins support.

The current way Merlin gets build information is too limited for practical uses nowadays and .merlin are an artifact of the past (... they were good enough when they were introduced, but the ecosystem started moving forward). Supporting jbuild is good for jbuilder but there are other systems out there.

I am thinking of a more general "log file" listing all compiler invocations in which Merlin pick the corresponding ocaml(c/opt) line but this is not the end of the story either (build systems tend to copy files, so the compiler view alone is too limited for Merlin).

@bschommer
Copy link
Contributor

How about using something similar to the clang compilation database. This works for pretty well for C/C++ tooling with clang.

@mroch
Copy link

mroch commented Oct 19, 2017

how about an INC config? so you could do INC _build/.merlin to get the behavior of this PR, and I could do something like INC ../../../buck-out/gen/.merlin to make this work with Buck.

@mroch
Copy link

mroch commented Oct 19, 2017

that is, rather than special-casing the resolution algorithm for each build system, continue to rely on the existing resolution algorithm. once it finds a .merlin file, that file can include the generated one from wherever it wants.

@ELLIOTTCABLE
Copy link
Contributor

It sounds like over-engineered intermediate solutions are a bad idea? The proposed change in the OP is a great hold-over, while something like clang's database can be hashed out with the build-tool people as the plan moving forward?

(I'm a newcomer to OCaml and have no idea what I'm talking about, so feel free to ignore me if there's a good reason for it — just been burned by too many hold-off measures with too many caveats in the past! 🤣)

@trefis
Copy link
Contributor

trefis commented May 16, 2018

#737 has been opened to discuss a potential support of json compilation database (as has also been suggested on this PR by bschommer).
I would like to keep the discussion in one place if possible, and rather there than here.

As for this PR:

  • it appears that we (maintainers of merlin) currently are not yet willing to offer the suggested behavior (look for .merlin files in a _build directory).
    We might change our mind depending on the (absence of) evolution of the alternative schemes proposed here and in JSON Compilation Database #737, but that might still take some weeks / months.
  • the implementation is incomplete.
    Currently, dune will generate a .merlin file in every directory where a library or executable is defined. If it ever starts output them in _build/ then the following should be kept in mind: the whole directory structure of the workspace is duplicated in _build for each of the build contexts, and it seems likely to me that .merlin files will be generated in the appropriate places in there, i.e. we won't have just a single _build/.merlin.
    It is still doable to look at the appropriate place in there, but it is more (tedious) work than what is done now.

Given that, I will close this PR, but feel free to reopen (a more complete) one in a few weeks if the situation hasn't changed at all.

Thanks!

@trefis trefis closed this May 16, 2018
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.

7 participants