better default unused face

[dradetsky]

The default unused face interacts poorly with scheme's extensive use of kebab-case. Reading the code

            (define-values (open-end-line open-end-col open-end-pos) (port-next-location in))

in which open-end-line and open-end-col were marked undefined, I found it hard to see how many variables there were, since the face strikethrough covered the kebab dashes. I mean, I was pretty sure it was 3 total, but it was annoying.

You can always customize this (I already have), but better defaults are better.

[greghendershott]

Thank you for submitting this.

Although it could work to use a wavy underline, a default color of "yellow" wouldn't be a good choice; that's effectively invisible with backgrounds like that of the Solarized Light theme.

A good default should (be very likely to) work with whatever else the user has chosen.

I think the status quo strike-through default is better at least in that respect, not depending on a color.

Also I really like the affordance: "crossing out" something that is unused and "N/A".

Also I don't find it ambiguous in the way that you report, because the strike-through line never goes through the space between two unused variables.

So... although I'm open to discussing more, I'm inclined not to make a change.

[sorawee]

Just wanna chime in to say that "interacts poorly with scheme's extensive use of kebab-case" is exactly why I made a similar change in my personal config, though with the orange color. I posted it in another issue: #638 (comment).

[greghendershott]

@sorawee Fair enough, but as for the default value:

I could see changing the default to be something like, "Draw a wavy underline using the same color specified by the user or user's theme for (say) font-lock-warning-face".

As if we could write something like:

(:underline (:color font-lock-warning-face :style wave))

Unfortunately it looks like the :color value for :underline must be an Emacs color name string. It can't be a face name. It has to be some hard-coded color (or, use the same as the foreground text color).

We could do

(:inherit font-lock-warning-face (:underline (:style wave)))

However that will draw both the text and the underline using that same color (overriding whatever colors the syntactic font-lock has applied for e.g. strings vs. variables vs. keywords etc.). Which I think is "too heavy handed". But maybe you and @dradetsky would feel that it's less-worse than the strike-through default.

[dradetsky]

@greghendershott

Also I don't find it ambiguous in the way that you report, because the strike-through line never goes through the space between two unused variables.

It's not literally ambiguous. It's just, as I say, annoying. For me, it requires extra brainpower to resolve the confusion about what the dashes mean. It's possible that I could learn to do this more easily, but I don't want to. I want to use that brainpower to write more terrible code. Also, I'd forget & get confused again if I stopped working extensively with racket.

a default color of "yellow" wouldn't be a good choice; that's effectively invisible with backgrounds like that of the Solarized Light theme.

What? What kind of cretin uses a non-dark backgro-I mean, good point, it's not a universally-usable default if we hardcode the color, good catch.

I could see changing the default to be something like, "Draw a wavy underline using the same color specified by the user or user's theme for (say) font-lock-warning-face".

Something like this sounds ideal, but I personally don't know enough about emacs faces to do it.

Also I really like the affordance: "crossing out" something that is unused and "N/A".

I really dislike it; I tend to cross things out when I finish them, so it sort of looks like emacs has taken care of all my mistakes for me, which it hasn't. In fact, it's just identified a bunch of possible mistakes that I should probably check. The original example comes from Racket's source, but normally if I have unused variables it usually means that my code is about to break when I try to run it. This is the sort of thing that should be highlighted to me, not crossed off like it's not relevant.

Unfortunately it looks like the :color value for :underline must be an Emacs color name string.

Presumably it doesn't literally have to be a color name string, like a literal, but just a form that evaluates to a color name string? So could we use the real-world equivalent of the following imaginary code?

(:underline (:color (get-face-attribute 'color font-lock-warning-face) :style wave))

That would probably work unless the user changed their theme. Which is a possibility if the user is using a light-background theme, since they might realize they should stop doing this and switch to a dark background theme LIKE A GODDAMN NORMAL PERSON. But at least we're getting closer to a solution.

[greghendershott]

Presumably it doesn't literally have to be a color name string, like a literal, but just a form that evaluates to a color name string? So could we use the real-world equivalent of the following imaginary code?

(:underline (:color (get-face-attribute 'color font-lock-warning-face) :style wave))

Unfortunately no, as I said, that's an error. It must be a literal string color name.

I've run into this before in Emacs. I wish it supplied a way for a face to lazily inherit some attributes of another face, but alas, you can't.

---

What could work is what I mentioned above; inherit from another face, and (say) add a wavy underline. Above I mentioned font-lock-warning-face, but actually the warning face would be a better choice. So:

(defface-racket racket-xp-unused-face
  '((t (:inherit warning :underline (:style wave))))
  "Face `racket-xp-mode' uses to highlight unused requires or definitions."
  "Unused Face")

This will draw both the text and the underline in the same color warning uses, so visually it's "heavier". (Which I don't love for this "weak" advisory that some definition or require is merely unused. Which is just my opinion.)

No default is going to be ideal for everyone. To some extent that's fine; Emacs' superpower is being able to customize nearly everything. I think the trick here is to pick the "least worst" default, whatever that might be.

[dradetsky]

Wow, that's weird. I'm guessing it's because defface calls custom-declare-face which appears to mainly do its work via

(face-spec-set face (purecopy spec) 'face-defface-spec)

so it basically never evaluates any variables in the spec.

While it's presumably possible to solve this by defining a new defface-racket-but-with-even-more-shitty-hacks-to-codewalk-n-eval-face-specs macro, I think what emacs wants us to actually do is something like

(defface-racket racket-xp-unused-face
  '((((background dark))
     (:underline (:color "yellow" :style wave)))
    (((background light))
     (:underline (:color "blue" :style wave))))

Unfortunately that literal thing doesn't work for me and I just get the default face. Not sure why; I checked (frame-parameters) and saw (background-mode dark) which I presume is what we're looking for.

[dradetsky]

@greghendershott anyway, if you know how to make that example work, I think that might be an improvement. If not, I think just telling people to customize it will have to do.

[greghendershott]

I think the basic issue is that defining a default value is something that occurs at compile time (at which point who knows what themes etc. are in effect), and also, a face spec default value doesn't allow using a symbolic reference or a thunk to get a specific face attribute "live". I don't know how to escape that box.

---

Yep, the dark vs. light background specification is something I already do use for example to define different shades of red for racket-keyword-argument-face.

In fact I started to explore this for racket-xp-unused-face. But I had trouble finding a shade of yellow that was both (a) visible and (b) non-vommity (term of art) against a Solarized light background.

I would ask you for help trying to find such a color, but I am already familiar with your views about working with light backgrounds. 😄

Seriously thanks for your feedback and I'll keep thinking about this.