add `DiagnosticTag`

[p00f]

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#diagnosticTag https://github.com/joaotavora/eglot/pull/794

DiagnosticTag provides additional metadata about a diagnostic, which till now means "unnecessary" and "deprecated"

[p00f]

[intramurz]

Interesting. Thank you for noticing this feature.

I've been thinking about your code and here's how it can be refactored (in my opinion).

First, you are directly matching faces as diagnostic tags, which is not great. I would suggest defining a function that takes a diags and returns the tags 'deprecated, 'unnecessary or nil. Inside, we derive the tags from the overlay properties.

Next, we calculate the level as usual. If we have diagnostic tags, we create a new error level on the fly from the current level and tags, and return it instead of the current one. We can copy all the fields from the existing level (there are suitable accessors in the flycheck API), and change only the :overlay-category. lsp-mode does something like that, look (but it would be better to do it simpler).

Finally, is it really necessary to create these new faces? Maybe it's possible to change the properties of the :overlay-category when we create the levels? In any case, they must be renamed to start with flycheck-eglot-, otherwise the emacs-lisp-package linter swears. And please write something in the docstrings other than a dot☺.

Then we would get rid of the boilerplate code and nested pcase. Such code will be shorter, cleaner and more resistant to changes.

What do you think of it? Good luck.

[p00f]

Sure, I'll try to do that

[p00f]

I still have to define the overlay-categories manually right?

[intramurz]

If you have good ideas on how to do it programmatically, give it a try. It would be great. The main criterion: if a new diagnostic tag has appeared, how much code will have to be added / rewritten (it is better to add than rewrite). I suppose when similar code is repeated many times, it would be better to reduce it somehow.

As for the rest, the simpler the better. The most important thing is that the code works.

[intramurz]

You are using the first function in line 181; it would be nice to use cl-first in cases like this.

[intramurz]

I think we can really take this as a prototype. I'm not sure if anonymous faces can be used in the context of an overlay category, that would be fine. It would also be nice not to depend on the dash library, but that's okay in principle.

[p00f]

I'm a novice at elisp and don't think I have the time to go through all of that, can you do it instead?

[intramurz]

I appreciate your modesty, and I'm sorry I scared you with my ideas and criticism. I think you gave up early. I also don't know everything and read a lot of help. In reality, everything is not as difficult as it seems. I would still like you to contribute as a co-author (because you were the first to notice the flaw and offer a working, albeit straightforward and slightly cumbersome solution). If you agree, we can split the work: you write a function flycheck-eglot--calculate-tags(diags) and a stub for flycheck-eglot--create-new-error-level(cur_level, tag)(let it always return cur_level) and change the way flycheck-eglot--report-eglot-diagnostics calculates level. I will then try to develop that stub to a working function. So you learn elisp and contribute to the development, and I write less code and get the first contributor☺.

If you still firmly decided to quit this venture, you could at least open an issue. Then I will close this PR.

Best wishes

[p00f]

Oh don't worry, it's just that elisp function names are very terse and non-descriptive so it is not comprehensible unless you know what they do.

If you agree, we can split the work: you write a function flycheck-eglot--calculate-tags(diags) and a stub for flycheck-eglot--create-new-error-level(cur_level, tag) (let it always return cur_level) and change the way flycheck-eglot--report-eglot-diagnostics calculates level

I can do that, thanks

[intramurz]

(Again) I read the eglot source, and concluded that tags should be a list. In the :overlay-properties of the eglot-generated diagnostics, the face field is actually a list of faces. Thus, there should be one tag for each face in this list.

[intramurz]

Finally, I had time and inspiration and I finished this work. Thanks for encouraging me to do this.