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

emacs' merlin-mode highlighting of code w/ errors is just bad #1663

Closed
UnixJunkie opened this issue Aug 23, 2023 · 11 comments · Fixed by ocaml/opam-repository#24882 or ocaml/opam-repository#24958

Comments

@UnixJunkie
Copy link

Putting a lot of code in a bold pinkish font is just not helpful.
That makes the whole code completely unreadable.
I have to disable merlin-mode so that I can continue
working on the code in question...

https://discuss.ocaml.org/t/emacs-merlin-mode-and-highlighting-of-code-w-errors/12845/2

@dbuenzli
Copy link

Add (setq merlin-error-on-single-line t) to your .emacs. Personally I think this should be the default.

@UnixJunkie
Copy link
Author

+1 for having nice defaults...

@3Rafal
Copy link
Collaborator

3Rafal commented Oct 23, 2023

@UnixJunkie ,
I made a PR that changes the defaults. Can you give me some nice examples of the behavior so that I can test it?

@UnixJunkie
Copy link
Author

Just take a significant ocaml source file from any project, and change it so that it does not compile anymore.

@UnixJunkie
Copy link
Author

And yes, this PR #1693 changes the default to something I prefer.
Now, the question is, is this what's preferred by most people or not and hence will the PR be merged...

@nojb
Copy link
Contributor

nojb commented Oct 23, 2023

Can you give me some nice examples of the behavior so that I can test it?

I exaplained a typical example that exhibits the issue in #1693 (comment)

@voodoos
Copy link
Collaborator

voodoos commented Oct 23, 2023

Now, the question is, is this what's preferred by most people or not and hence will the PR be merged...

I personally don't have a strong opinion about this. Knowing the scope of the error is useful, but having the whole buffer turn red is not. Do other language plugins have better way to deal with this issue ? Maybe we could underline everything only when more information is asked about the error ?

Since we had recently a nice collective discussion about keybindings, I will summon the people who took part in it to gather more feedback 🙂

@Chris00 @mattiase @bbatsov @erikmd @catern

@bbatsov
Copy link

bbatsov commented Oct 23, 2023

Depends on what info we can get from the compiler about the error - in CIDER we try to map the errors to a single expression, even if it's multiline. Can someone post a screenshot of the problem, because I don't recall seeing crazy error highlighting in Merlin.

@nojb
Copy link
Contributor

nojb commented Oct 23, 2023

Can someone post a screenshot of the problem, because I don't recall seeing crazy error highlighting in Merlin.

image

@bbatsov
Copy link

bbatsov commented Oct 24, 2023

Yeah, this looks pretty bad indeed. I think most packages would just underline characters 2-19 on line 121 as indicated by the compiler error.

@mattiase
Copy link
Contributor

The wholesale highlighting of the entire module struct because of a single mismatch is rubbish of course, but setting merlin-error-on-single-line doesn't seem to be the solution. The text becomes more readable in this case but at the price of making the highlighting quite pointless.

Anything preventing us from highlighting the ranges supplied by the error messages? They seem a lot more informative, as in

File "broken.ml", line 3, characters 2-16: Expected declaration
File "broken.ml", line 6, characters 6-7: Actual declaration

voodoos added a commit to voodoos/opam-repository that referenced this issue Dec 1, 2023
CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
voodoos added a commit to voodoos/opam-repository that referenced this issue Dec 1, 2023
CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Ignore the new 5.1 `cmi-file` flag instead of rejecting it (ocaml/merlin#1710, fixes
      ocaml/merlin#1703)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
voodoos added a commit to voodoos/opam-repository that referenced this issue Dec 1, 2023
CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Ignore the new 5.1 `cmi-file` flag instead of rejecting it (ocaml/merlin#1710, fixes
      ocaml/merlin#1703)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
    - Add support for OCaml 5.1.1 (ocaml/merlin#1714)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
voodoos added a commit to voodoos/opam-repository that referenced this issue Dec 18, 2023
CHANGES:

Mon Dec 18 16:42:00 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
voodoos added a commit to voodoos/opam-repository that referenced this issue Dec 18, 2023
CHANGES:

Mon Dec 18 16:42:00 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Ignore the new 5.1 `cmi-file` flag instead of rejecting it (ocaml/merlin#1710, fixes
      ocaml/merlin#1703)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
    - Add support for OCaml 5.1.1 (ocaml/merlin#1714)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)

[new release] merlin-lib and merlin (4.13-501)

CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Ignore the new 5.1 `cmi-file` flag instead of rejecting it (ocaml/merlin#1710, fixes
      ocaml/merlin#1703)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)

[new release] merlin-lib and merlin (4.13-414)

CHANGES:

Fri Dec  1 15:00:42 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

Mon Dec 18 16:42:00 CET 2023

  + merlin binary
    - Fix a follow-up issue to the preference of non-ghost nodes introduced in ocaml/merlin#1660 (ocaml/merlin#1690, fixes ocaml/merlin#1689)
    - Add `-cache-lifespan` flag, that sets cache invalidation period. (ocaml/merlin#1698,
      ocaml/merlin#1705)
    - Fix Merlin locate not fallbacking on the correct file in case of ambiguity
      (@goldfirere, ocaml/merlin#1699)
    - Fix Merlin reporting errors provoked by the recovery itself (ocaml/merlin#1709, fixes
      ocaml/merlin#1704)
  + editor modes
    - vim: load merlin when Vim is compiled with +python3/dyn (e.g. MacVim)
    - emacs: highlight only first error line by default (ocaml/merlin#1693, fixes ocaml/merlin#1663)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants