eclipselabs/eclipse-language-service

As you type validation should use annotation instead of marker

Closed this issue · 9 comments

As you type validation should use annotation instead of marker with IAnnotaionModel.

Can you please elaborate why?

  • IMarker is used to have persistent error (you can see errors in the Problem View)
  • IAnnotationModel is used to mark errors when you are typing

Java, JavaScript, XML, JSON, etc editors work like this:

  • when you are typing, it uses IAnnotationModel to mark errors. The benefit with IAnnotationModel is that :
    1. it higlight content where there are errors.
      -2) errors are not persistent. Errors are not displayed in the problem view
  • when you save your Java, JavaScript Editor, a builder is executed and mark errors with IMarker to have persistent errors and your errors are displayed in the problem view.

In other words:

  • use IMarker with builder.
  • use IAnnotationModel when you are typing.
1) it higlight content where there are errors.

With the current problem markers, the errors are underlined and visible too.

-2) errors are not persistent. Errors are not displayed in the
problem view

I see it as a drawback, I like to have all errors in the Problems View,
even if the document isn't saved yet.

when you save your Java, JavaScript Editor, a builder is executed
and mark errors with IMarker to have persistent errors and your
errors are displayed in the problem view.

Here, we don't have builders, it's the language server that returns
diagnostics, and language server has no concept of save or not save,
builders vs as-you-type.

In other words:

  • use IMarker with builder.
  • use IAnnotationModel when you are typing.

I'll wait for some end-user to give feedback on this topic, as so far, I
see no real benefit in using IAnnotationModel rather than markers. Also,
setting up quick-fix for markers is quite simple (but maybe it's the
same with IAnnotationModel).

Closing it as I'm failing to find the value of this proposal. But that can be reconsidered later if it shows some benefits in user experience.

@mickaelistria I'm really sorry to insist, but IMHO I think we should use annotations.

Here the result with your Generic editor & marker:

jsonwithmarker

Here the result with WTP JSON Editor & annotation::

jsonwithannotation

As you can see, it higlight the errors and you can benefit with popup.

I will try to do a PR if I find time.

As you can see, it higlight the errors

What you're reporting here isn't related to annotations vs markers, it just highlight a potential bug in the language server, in the diagnostic to marker converter or in the editor itself. If you look as some C# code with OmniSharp language server, you see the erroneous code underlined as expected.

and you can benefit with popup.

That's an interesting thing. If this doesn't show up with Problem markers, I suggest we make it an enhancement request to the Generic Editor directly.

Markers have very high value: (upcoming) deep integration in Project Explorer, easy to plug remediation, shown in Problems view... Any way to improve Problems support is welcome, but really the language server protocol won't adopt IAnnotationModel rather than markers.
But as mentioned, it can be an enhancement request for Platform or Generic Editor if it happens that pop-up isn't shown.

I will try to do a PR if I find time.

Don't waste your time making a PR against this project as I'm still sure that Problem markers are the concept we need to use there, so I won't approve a change that goes against using Problem markers to report problems. General issues about reporting of Problems in text editors should be reported to Eclipse Platform.

Ok thanks @mickaelistria for your answer. I stop to insist:)

As you can see, it higlight the errors

Good catch! It's https://bugs.eclipse.org/bugs/show_bug.cgi?id=503326

and you can benefit with popup.

And this is https://bugs.eclipse.org/bugs/show_bug.cgi?id=503332