Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that face-background reads indirect values #179

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

protesilaos
Copy link
Contributor

The old design would return an inappropriate nil value if the referenced face did not have an explicit background attribute. This can happen if a theme uses ':inherit' instead of specifying the face attributes directly.

By declaring a fallback value, we make sure that such indirection will note return a nil value.

This is related to issue 178: #178.

The old design would return an inappropriate nil value if the referenced
face did not have an explicit background attribute.  This can happen if
a theme uses ':inherit' instead of specifying the face attributes
directly.

By declaring a fallback value, we make sure that such indirection will
note return a nil value.

This is related to issue 178:
<ema2159#178>.
@CsBigDataHub
Copy link

Hi @ema2159,

Can you please have a look and push this PR!

Thanks

@CsBigDataHub
Copy link

Hi @nebhrajani-a, @ema2159 ,

Tested this PR and it fixed #187 .

Setting both options to t still causes Invalid face attribute :foreground nil.

(setq centaur-tabs-set-icons t)
(setq centaur-tabs-plain-icons t)

Just setting one option fixed it. Went with (setq centaur-tabs-set-icons t).

@protesilaos
Copy link
Contributor Author

Setting both options to t still causes Invalid face attribute :foreground nil.

That would mean there are more cases where a fallback value needs to be specified. I can do it, if you want, but let's wait for this to be finalised first.

@CsBigDataHub
Copy link

@ema2159 friendly ping.

protesilaos added a commit to protesilaos/modus-themes that referenced this pull request Aug 2, 2022
centaur-tabs has a bug where it cannot read the value of a face if it
uses the standard ':inherit' attribute.  I have sent a patch to fix it,
but have received no response since February:
<ema2159/centaur-tabs#179>.

Relevant reports:

- <#30>
- <https://gitlab.com/protesilaos/modus-themes/-/issues/288>
- <#15>

I am happy to reinstate support for centaur-tabs as soon as the package
gets the maintenance it needs.
@ema2159
Copy link
Owner

ema2159 commented Sep 19, 2022

Hey! I am back. Will review and push soon. Shouldn't take long.

@ema2159 ema2159 merged commit 40e337a into ema2159:master Sep 20, 2022
@ema2159
Copy link
Owner

ema2159 commented Sep 20, 2022

Thanks a lot for your contribution, and sorry for the delay.

@protesilaos
Copy link
Contributor Author

@ema2159 You are welcome!

@ema2159
Copy link
Owner

ema2159 commented Sep 20, 2022

Also, eac6522 solves the issue @CsBigDataHub pointed out.

protesilaos added a commit to protesilaos/modus-themes that referenced this pull request Sep 20, 2022
I had removed support for centaur-tabs in commit 2235ce5 (done on
2022-08-02).  I wrote:

    centaur-tabs has a bug where it cannot read the value of a face if it
    uses the standard ':inherit' attribute.  I have sent a patch to fix it,
    but have received no response since February:
    <ema2159/centaur-tabs#179>.

    Relevant reports:

    - <#30>
    - <https://gitlab.com/protesilaos/modus-themes/-/issues/288>
    - <#15>

    I am happy to reinstate support for centaur-tabs as soon as the package
    gets the maintenance it needs.

My patch/pull-request is now merged and the package is actively
maintained once again.  Hence the decision to bring back support for it,
as promised.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants