search

flycheck / flycheck-ocaml

OCaml support for Flycheck using Merlin
GNU General Public License v3.0
22 stars 6 forks source link

Enable only when .merlin file is present? #11

Closed edwintorok closed 1 year ago

edwintorok commented 5 years ago

In doom-emacs I've activated flycheck-ocaml only when a .merlin file is present in the project, and intend to do the same in Spacemacs.

Should this be done in flycheck-ocaml itself?

The rationale is that if there is no .merlin file you will likely get a lot of errors reported because merlin won't be able to find the include paths for all the modules that you use and you'd end up turning off flycheck anyway. A .merlin file is present if the jbuilder/dune build system is used, or if a project manually supplies one (they used to do that before jbuilder appeared).

rgrinberg commented 5 years ago

You're checking for the .merlin relative to the source file that is being syntax checked, right?

edwintorok commented 5 years ago

Yes it looks in the same directory as the source file, and then goes up the directory tree, if it finds a .merlin it activates flycheck: (when (and buffer-file-name (locate-dominating-file buffer-file-name ".merlin"))

bbatsov commented 1 year ago

Let's see if I managed to get this right. Haven't worked on flycheck in ages!

bbatsov commented 1 year ago

Ha, it seems that by the time I implemented this it's no longer relevant https://tarides.com/blog/2021-01-26-recent-and-upcoming-changes-to-merlin :-)

lthms commented 1 year ago

The .merlin file is not necessary for a long time now, when you use dune, and the project I am working on typically don’t have one. So I had to remove the check for flycheck-ocaml to work again as intended.

bbatsov commented 1 year ago

@lthms I've already removed it here as well and I replaced it with a check for dune-project.

drvink commented 1 year ago

FYI, this is both unnecessary (merlin definitely does not require dune, nor is a .merlin file strictly required) as well as annoying (because now flycheck will refuse to check files that merlin itself could check fine; for example, a standalone .ml script, or a file that uses other modules and .cmis are present). I recommend removing the check for dune-project or .merlin entirely.

bbatsov commented 1 year ago

@drvink I get that's not strictly necessary, but based on my experience you don't get anything meaningful from Merlin without some setup for it. Admittedly, I'm far from a Merlin expert, but if we remove this check we'd still need some meaningful way to tell the users what's missing on their end.

drvink commented 1 year ago

I think it's good enough to just show whether a .merlin and/or dune-project file was found in flycheck-verify-setup rather than refusing to run the checker.