* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic
@ 2022-09-28 11:39 Augusto Stoffel
2022-10-08 11:15 ` Augusto Stoffel
2023-02-18 15:00 ` bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic' Augusto Stoffel
0 siblings, 2 replies; 6+ messages in thread
From: Augusto Stoffel @ 2022-09-28 11:39 UTC (permalink / raw)
To: 58141; +Cc: João Távora
[-- Attachment #1: Type: text/plain, Size: 72 bytes --]
Or at least I really dislike the default value, which looks like this:
[-- Attachment #2: Screenshot from 2022-09-28 13-16-40.png --]
[-- Type: image/png, Size: 6092 bytes --]
[-- Attachment #3: Type: text/plain, Size: 599 bytes --]
When I first saw this face appearing, I had no idea what was going on.
That this is a Flymake warning is unexpected, since there's nothing
squiggly about it. Moreover, it overrides all other font-locking, which
presumably still has meaningful information. Finally, I think it's hard
to be sure the result is readable in all custom themes, even all
reasonable ones.
In general, I would argue that “creative” decorations should be left off
by default. So concretely in this case my suggestion would be to let
eglot-diagnostic-*-face inherit from one of the good old Flymake faces.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic
2022-09-28 11:39 bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic Augusto Stoffel
@ 2022-10-08 11:15 ` Augusto Stoffel
2023-02-18 15:00 ` bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic' Augusto Stoffel
1 sibling, 0 replies; 6+ messages in thread
From: Augusto Stoffel @ 2022-10-08 11:15 UTC (permalink / raw)
To: 58141; +Cc: João Távora
On Wed, 28 Sep 2022 at 13:39, Augusto Stoffel wrote:
> Finally, I think it's hard to be sure the result is readable in all
> custom themes, even all reasonable ones.
I initially wrote this as an abstract argument, but let me point out
that in the Modus Operandi theme (which is eminently reasonable), the
shadow face and font-lock-comment-face are equal. So it's hard to
distinguish commented-out code from (presumably) unnecessary code.
Here's a concrete suggestion: when a language server diagnostic says
some code is "unnecessary", then apply _both_
eglot-diagnostic-tag-unnecessary-face and the usual Flymake face, with
that order of priority. If the user so wishes, they can use
eglot-diagnostic-tag-unnecessary-face to override any attribute of the
flymake-* faces. But, by default, the eglot-diagnostic-tag-* faces are
best kept empty.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic'
2022-09-28 11:39 bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic Augusto Stoffel
2022-10-08 11:15 ` Augusto Stoffel
@ 2023-02-18 15:00 ` Augusto Stoffel
2023-02-23 11:59 ` João Távora
1 sibling, 1 reply; 6+ messages in thread
From: Augusto Stoffel @ 2023-02-18 15:00 UTC (permalink / raw)
To: arstoffel; +Cc: joaotavora, 58141
[-- Attachment #1: Type: text/plain, Size: 182 bytes --]
This was a while back, but in any case this is my suggestion for this
face. I haven't seen eglot-diagnostic-tag-deprecated-face appear in
nature so I'm not sure how well it works.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-eglot-diagnostic-tag-unnecessary-face.patch --]
[-- Type: text/x-patch, Size: 987 bytes --]
From 0772f1b46af4e1745d417db0a62b6de64575face Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sat, 18 Feb 2023 15:58:23 +0100
Subject: [PATCH] Improve eglot-diagnostic-tag-unnecessary-face
* progmodes/elgot.el (eglot-diagnostic-tag-unnecessary-face): Use wavy
underline, as in all other Flymake faces.
---
lisp/progmodes/eglot.el | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 6d192d9b333..8075412411e 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -324,7 +324,10 @@ eglot-mode-line
"Face for package-name in Eglot's mode line.")
(defface eglot-diagnostic-tag-unnecessary-face
- '((t (:inherit shadow)))
+ '((((supports :underline (:style wave)))
+ :underline (:style wave :color "gray"))
+ (t
+ :inherit flymake-note))
"Face used to render unused or unnecessary code.")
(defface eglot-diagnostic-tag-deprecated-face
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic'
2023-02-18 15:00 ` bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic' Augusto Stoffel
@ 2023-02-23 11:59 ` João Távora
2023-02-23 18:55 ` Augusto Stoffel
0 siblings, 1 reply; 6+ messages in thread
From: João Távora @ 2023-02-23 11:59 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 58141
On Sat, Feb 18, 2023 at 3:00 PM Augusto Stoffel <arstoffel@gmail.com> wrote:
>
> This was a while back, but in any case this is my suggestion for this
> face. I haven't seen eglot-diagnostic-tag-deprecated-face appear in
> nature so I'm not sure how well it works.
I'm not a fan of hardcoding colors in Eglot, since that needs themes
to update for the new face. Supporting the optional underline is
probably fine, though.
João
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic'
2023-02-23 11:59 ` João Távora
@ 2023-02-23 18:55 ` Augusto Stoffel
2023-02-23 19:10 ` João Távora
0 siblings, 1 reply; 6+ messages in thread
From: Augusto Stoffel @ 2023-02-23 18:55 UTC (permalink / raw)
To: João Távora; +Cc: 58141
On Thu, 23 Feb 2023 at 11:59, João Távora wrote:
> I'm not a fan of hardcoding colors in Eglot, since that needs themes
> to update for the new face. Supporting the optional underline is
> probably fine, though.
Okay, but the current choice (inherit from shadow) is also not a safe
one with respect to themes. So I'd say the next best thing is to make
that face inherit from flymake-note or flymake-warning.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic'
2023-02-23 18:55 ` Augusto Stoffel
@ 2023-02-23 19:10 ` João Távora
0 siblings, 0 replies; 6+ messages in thread
From: João Távora @ 2023-02-23 19:10 UTC (permalink / raw)
To: Augusto Stoffel; +Cc: 58141
On Thu, Feb 23, 2023 at 6:55 PM Augusto Stoffel <arstoffel@gmail.com> wrote:
>
> On Thu, 23 Feb 2023 at 11:59, João Távora wrote:
>
> > I'm not a fan of hardcoding colors in Eglot, since that needs themes
> > to update for the new face. Supporting the optional underline is
> > probably fine, though.
>
> Okay, but the current choice (inherit from shadow) is also not a safe
> one with respect to themes.
That's a problem with theme. The default theme has a good setting
for "shadow", in my opinion.
João
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-23 19:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 11:39 bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic Augusto Stoffel
2022-10-08 11:15 ` Augusto Stoffel
2023-02-18 15:00 ` bug#58141: [Eglot] Default eglot-diagnostic-tag-unnecessary-face is problematic' Augusto Stoffel
2023-02-23 11:59 ` João Távora
2023-02-23 18:55 ` Augusto Stoffel
2023-02-23 19:10 ` João Távora
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).