ocaml-lsp icon indicating copy to clipboard operation
ocaml-lsp copied to clipboard

false negatives on unsaved files

Open EduardoRFS opened this issue 3 years ago • 4 comments
trafficstars

Currently if you have an unsaved file that previously had an error and the error was removed but the file is still not saved ocamllsp show errors that don't exist anymore coming from dune.

A solution would be to disable dune errors on unsaved files.

EduardoRFS avatar May 27 '22 18:05 EduardoRFS

I've thought about it a bit and the heuristic above doesn't seem to work well. For example, suppose the user is editing foo.ml and dune reporting an ml/mli mismatch between foo.ml and foo.mli. Immediately removing the error as the user is typing would make it hard to fix this error.

rgrinberg avatar May 30 '22 21:05 rgrinberg

Fair, can we get a flag to disable dune errors then?

EduardoRFS avatar May 31 '22 00:05 EduardoRFS

If you don't want to see dune errors, why do you use the watch mode then? You can remap your C-s key to save followed by $ dune build and get the user experience you want.

We should try a little harder to solve this problem. Another approach would be to remove all syntax/type errors that satisfy:

  • Come from dune
  • Are typing errors
  • All with locations inside the edited document

The only issue is that the compiler isn't useful enough to tell us what are type/syntax errors.

rgrinberg avatar May 31 '22 00:05 rgrinberg

I really don't think questioning on why I want my user experience back is the right way to think about it, this was a regression in many ways.

Making C-s runs dune build is definitely not the same, as it's not as fast and I would need to configure a different key binding as sometimes I'm running dune build -w @check others @check @runtest also I may have some part of the project and the remaining working, doing that would mean every time I save I will get all the old errors again.

And that still doesn't change the fact that it's showing me dune errors on my OCaml file, I don't know why you wanted it, but I don't, if the goal was to achieve mli conflicts showing on the file this was a hackish solution and I'm not sure even if that is desirable, a lot of times I first update the mli and then keep hacking on the ml until I decide it's good enough and save to check with dune if that's okay, now I'm coding for hours with my entire file in red.

There is also the duplicated errors and the outdated errors, plus dune may be running with different presets than what I want from merlin.

Edit

This for me seems to be a classical case of having more than one source of truth which is rarely a good idea. And as the only job of the LSP is to improve dev experience, having false negatives everywhere seems clearly undesirable.

Edit 2

A simple patch if someone else needs it.

https://github.com/EduardoRFS/ocaml-lsp/commit/eeb5ff15c6a3e78c897c6f787ce8ebb4ffe10934

EduardoRFS avatar May 31 '22 04:05 EduardoRFS

@rgrinberg can we get a flag to disable those? I have someone to implement that, I just need to know how that would look like

OCaml 4.14

https://github.com/EduardoRFS/ocaml-lsp/commit/06a7589b09407478bbea0eb8952e42ff4c77113f

OCaml 5.0

This one is a bit outdated, but still works, I've been using it for almost an year.

https://github.com/EduardoRFS/ocaml-lsp/commit/eeb5ff15c6a3e78c897c6f787ce8ebb4ffe10934

EduardoRFS avatar May 19 '23 15:05 EduardoRFS

Sure, why not.

rgrinberg avatar May 20 '23 21:05 rgrinberg