jwiegley / use-package

A use-package declaration for simplifying your .emacs
https://jwiegley.github.io/use-package
GNU General Public License v3.0
4.4k stars 260 forks source link

:inherit for :custom-face no more works same way #1008

Closed tttuuu888 closed 1 year ago

tttuuu888 commented 1 year ago

First of all, I would like to appreciate to @jwiegley and every contributors for great effort for this great package.

Recent pull request #1004 has changed how :custom-face working. Previously :inherit overwrite all attributes of current face but now :inherit merge attributes to current face if the attribute of current face is unspecified. This matches description of face-spec-set but people, including me who were using :inherit in previous way lost way to set face using :custom-face.

On the other hand, face-spec-set has optional argument SPEC-TYPE and if 'face-defface-spec' is given, it works as same as previous :custom-face. But I couldn't find a way to give this optional SPEC-TYPE argument to :custom-face.

So I was wondering if there is already a way to give a SPEC-TYPE argument to :custom-face, or if it doesn't, it can support this feature.

Thank you.

andreyorst commented 1 year ago

@tttuuu888 IIUUC you can do this:

(use-package foo
  :custom-face
  (foo-face ((t (:inherit unspecified))) face-defface-spec))
andreyorst commented 1 year ago

Ah, no, my mistake, it won't work this way

andreyorst commented 1 year ago

@tttuuu888 Can you check if this patch works for you? https://patch-diff.githubusercontent.com/raw/jwiegley/use-package/pull/1009.patch

Also, if you could provide a minimal example code of what has changed so perhaps we could add a test for this?

tttuuu888 commented 1 year ago

Hello @andreyorst, Thank you so much. https://patch-diff.githubusercontent.com/raw/jwiegley/use-package/pull/1009.patch works perfectly!

Since the patch works, test code might not be a necessary but I made a simple example for this case.

(use-package emacs
  :custom-face (region ((t :inherit mode-line-inactive))))

You can see all attributes of region face are inherited(overwritten) from mode-line-inactive with previous use-package but it doesn't with current use-package.

And after applying the patch, I was able to make it work the same as the previous use-package with the code below.

(use-package emacs
  :custom-face (region ((t :inherit mode-line-inactive)) face-defface-spec))

I think this issue can be closed as soon as the pull request is merged.

Thank you again.

andreyorst commented 1 year ago

@jwiegley I wonder if instead of supplying the third argument we should just set the spec-type to face-defface-spec by default. WDYT?

jwiegley commented 1 year ago

@andreyorst I've merged in the fix, but open to changes!