Org-mode tags in org-modern-tag-faces get spell-checked

[andersjohansson]

Hi,

Tags that are listed in org-modern-tag-faces get a face definition that is not excluded by the rules in jinx-exclude-faces.

If I do:

(setq org-modern-tag-faces   '(("ignoreheading" modus-themes-nuanced-blue)))

An "ignoreheading" tag gets the face:

(:inherit ((modus-themes-nuanced-blue) org-modern-tag))

And this is not matched in jinx--face-excluded-p.

I also tried doing this for all the faces I added to org-modern-tag-faces:

(cl-pushnew '(:inherit (modus-themes-nuanced-blue) org-modern-tag)
                        (alist-get 'org-mode jinx-exclude-faces))
            (add-to-list 'org-modern-tag-faces (list name face))

But this doesn’t match since it won’t be eq (but it is equal). Extracting the central check in jinx--face-excluded-p, this returns nil:

(memq '(:inherit ((modus-themes-nuanced-blue) org-modern-tag))
       (alist-get 'org-mode jinx-exclude-faces))

I don’t know what a smart solution for this would be, but since both packages are yours I gather you can come to a good decision (or decide to just not support this case, which could also be reasonable).

[minad]

I haven't customized org-modern-tag-faces, so I haven't come across this problem so far. I think you should simply configure jinx-exclude-faces appropriately.

[minad]

Hmm, so the problem is that jinx--excluded-p does not take inherited faces into account.

EDIT: I pushed a fix to org-modern, such that no intermediate face with :inherit is created, see
minad/org-modern@567b702. This way no additional jinx-exclude-faces customization is needed, since org-modern-tag is already ignored.

I don't recall why I had used :inherit in the first place, maybe I had some misconception about merging faces, when I wrote this org-modern code. Let's see if this causes other problems in org-modern.

Does this work for you?

[andersjohansson]

Thank you! This works for me in initial testing and seems like a good solution. I suppose other packages could set "complex" face definitions like that, but probably it is not really needed, and nothing that jinx should add support for.

[minad]

I suppose other packages could set "complex" face definitions like that, but probably it is not really needed, and nothing that jinx should add support for.

Yes, I think complex face definitions are often a mistake, like in this case in org-modern. The Jinx face exclusion is about excluding semantic faces, so it is reasonable to support symbolic face names only. For complex scenarios Jinx also supports custom predicate functions or jinx--property-excluded-p.