search

dajva / rg.el

Emacs search tool based on ripgrep
https://rgel.readthedocs.io
GNU General Public License v3.0
474 stars 39 forks source link

fix font-lock-face for rg-toggle-{on,off}-face #36

Closed qwerbzuio closed 6 years ago

qwerbzuio commented 6 years ago

The faces rg-toggle-{on,off}-face were not processed properly. Apparently only the car of the list assigned to font-lock-face is being considered.

If this is true, also some other settings for font-lock-face won't have the desired effect (i.e. the cdr of the lists get discarded, which mostly means that the 'bold' property is ignored).

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 82.535% when pulling 51dc3b46ffa6f545ab2dbb05e78b2ac7e3707768 on qwerbzuio:master into d50bd106275f3ef7f77d0147857412fb065eef47 on dajva:master.

qwerbzuio commented 6 years ago

I'm using version 25.3.1. But, indeed I didn't check before with emacs -Q. Sorry. The issue indeed disappears then. So I'll have to find out what's wrong with my configuration. As to the documentation of font-lock-face, I could only find a reference here. No mention of a list is made there. Could you kindly point me to your reference?

dajva commented 6 years ago

That's my reference as well. It says that:

This property specifies a value for the face property that Font Lock mode should apply to the underlying text.

which I interpret as it has the same format as the face property, which could be a list of faces according to the same page. Not super clear but I think it make sense that you can use the same format.

The rg faces inherits from other faces. Wonder if it's something in your config that overrides some parent face possibly. Please let me know if there is some issue with how the faces in this package is defined.

qwerbzuio commented 6 years ago

Turns out that my color-theme setup was broken: the face bold had the color set to black. This is the leuven theme, which is probably to blame for the issue, as a face bold shouldn't affect the color (in fact, I had leuven combined with a primary dark color-scheme, which rendered the black on/off-text unreadable).

To make rg.el more robust against such theme-settings, I'd propose to write - (propertize value 'font-lock-face `(bold ,face)) + (propertize value 'font-lock-face `((:weight bold) ,face))

But, in principle, it's not rg.el's fault here. I'll close the requrest.