dradetsky
commented
5 years ago the other problem with this version is that now it's not clear why aw-minibuffer-leading-char-face is defined in ace-window in the first place. In the original version, it makes sense: we're going to use that face if the flag is set, so it has to be defined. In this version, for the majority of users, this change means a slight increase in memory consumption with no effect. If we do it this way, it seems like it should be the user's responsibility to define a new custom face if he wants a separate minibuffer face, and then set the pointer to this face. But this substantially increases the configuration difficulty. Now I have to add a defface to my init. The typical user will probably do what I did to implement it: open up the source code an yank a copy of aw-leading-char-face. But this doesn't seem like the ideal configuration workflow to me.
Maybe I'm not very imaginative, but my use case for this pr is just about the only one I can think of. If so, then the configuration required is just setting 1 boolean. In addition, for most users, the default face is fine for minibuffers, they only want to modify the regular face without affecting the minibuffer, so it's very straightforward.
Per request in #180, did this differently. I don't like this version as much because:
the reasons i mentioned in #180.
the documentation is now less straightforward.
i'm not even sure i'm doing the
defcustomform correctly. is:type 'facethe right custom widget? It shows a preview, but you just edit the value as text rather than getting some kind of select-from-available-faces ui (not that I ever use customize myself, but apparently people do).But it works the same as the other version.