Closed dradetsky closed 5 years ago
Thanks. I think it's better to have aw-minibuffer-separate-face
point to 'aw-leading-char-face
. Then the user can customize it to point to 'aw-minibuffer-leading-char-face
or even something else.
Could you please update the PR?
I mean, if you want me to do it that way, I will. I thought of that, but didn't do it originally because:
the existing code doesn't work that way. we don't have (defvar aw-face-for-leading-char aw-leading-char-face)
. So it's inconsistent.
the user can still change the minibuffer face to anything else with resetting or face-set-spec
or whatever. this is kind of esoteric (i just assumed it existed, but i had to look it up), but the user would have to do this if he wants to set aw-leading-char-face
to an arbitrary face, since that's just a face, not a variable.
i didn't actually profile or anything, but it seemed intuitively better to do (if (and false x) c a)
, rather than (if x (foo a) (foo b))
where a = b
, when we're deep in an inner loop. but this isn't a serious consideration, and i didn't profile, so this doesn't really matter.
But this is your repo, not mine, and you're clearly a much better elisp programmer than me. So i'm going to do it that way too. But maybe teach me something: why do you want it that way?
Thanks. I rewrote your commit with some simplifications. Please test if it still works.
Generally, I prefer features to have more flexibility. Often, that means avoiding boolean flags. However, in this case, it's appropriate to just have a separate face and have the users / themes customize that.
Also, only the most important customization should go into README. The user can discover the rest with M-x customize
.
@abo-abo yep, works fine. I agree your rewrite is better. I'm just superstitious about changing APIs i guess.
i use ace-window in a setup with many monitors and many separate frames. as a result, i found it useful to modify
aw-leading-char-face
so it was huge and easy to see. the problem was that if i ended up switching between windows while using the minibuffer, i would try to switch back to it, but be unable because i couldn't see the letter i was supposed to press to switch to it. and with so many possibilities, working out the letter it might be was too much trouble.so instead, i define a separate face, which the user can optionally set to use for minibuffers. i can leave this one the normal size, and the problem goes away.