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

Check for errors when opening new ocaml files in Emacs #880

Closed
wants to merge 1 commit into from

Conversation

Wilfred
Copy link
Contributor

@Wilfred Wilfred commented Nov 9, 2018

Otherwise, the user has to explicitly save the file before they can
see any merlin errors.

Otherwise, the user has to explicitly save the file before they can
see any merlin errors.
@trefis
Copy link
Contributor

trefis commented Nov 12, 2018

I don't think this has its place in merlin's default configuration:

  • it's dubious that everyone would want this: it can slow down the opening of new files, it can be very noisy when you're opening a file in a project that isn't compiled yet, ...
  • unless I'm mistaken, it's very easy to update your .emacs to have that behavior

Thank you for the suggestion though!

@trefis trefis closed this Nov 12, 2018
@rgrinberg
Copy link
Member

I also agree that this seems like the wrong way to accomplish this. Is there a flycheck option to check a buffer after opening it? That would solve the problem in general.

@Wilfred
Copy link
Contributor Author

Wilfred commented Nov 12, 2018

Flycheck and flymake always check the buffer when you open the file, so that set my expectations. I also found it surprising that if I explicitly call M-x merlin-mode in a buffer, I don't see any merlin errors until I save.

It's possible to work around this in my own Emacs configuration, but my concern is that it's not discoverable. Have I missed something?

I'm less concerned about performance: I've tested this on an ocaml codebase with >100KLOC and >100 errors and it wasn't an issue. Flycheck is actually more aggressive than merlin: most checkers will update even if you don't save.

I suppose it's a question of whether you see merlin.el as primarily a code completion tool, or an error display package :)

@jberdine
Copy link

FWIW, maybe it's just me, but I not infrequently open files and explicitly do not want merlin to report errors, since the file has not been compiled, is at the other end of an only minimally-configured tramp connection, etc.

@rgrinberg
Copy link
Member

Flycheck and flymake always check the buffer when you open the file, so that set my expectations. I also found it surprising that if I explicitly call M-x merlin-mode in a buffer, I don't see any merlin errors until I save.

So why doesn't it work in this case for you? Flycheck should drive this via https://github.com/flycheck/flycheck-ocaml. My main concern is such a setting shouldn't really be chosen at the mode level. It would be annoying to set this for ocaml, python, haskell, etc.

@Wilfred
Copy link
Contributor Author

Wilfred commented Nov 14, 2018

@jberdine FWIW I've been thinking of adding tramp support for Merlin, which would suit my workflow and maybe yours too?

@rgrinberg aha, I didn't realise that flycheck supported merlin! I see there's flycheck-ocaml which teaches flycheck how to use merlin. I only had flycheck itself.

@Wilfred
Copy link
Contributor Author

Wilfred commented Nov 14, 2018

I've added a note to the wiki page. Thanks for clarifying :)

@Wilfred Wilfred deleted the check-for-errors-on-open branch November 14, 2018 15:35
@jberdine
Copy link

jberdine commented Nov 14, 2018 via email

@Wilfred
Copy link
Contributor Author

Wilfred commented Jan 10, 2019

I did explore using flycheck with merlin, but it doesn't handle error ranges. It can't handle highlighting the part of the line that has the type error (see flycheck/flycheck-ocaml#12 and flycheck/flycheck#1400). At this point, I think merlin.el works better without flycheck.

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.

None yet

5 participants