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

markdown-code-face definition ignored #241

Closed
blaenk opened this issue Aug 24, 2017 · 3 comments
Closed

markdown-code-face definition ignored #241

blaenk opened this issue Aug 24, 2017 · 3 comments
Labels

Comments

@blaenk
Copy link

blaenk commented Aug 24, 2017

Sorry for the title. What I mean is that I'm styling markdown-code-face in my theme but it seems to be ignored. Eventually I found markdown-update-code-face which seems to check if it's been defined and if not then produces its own definition. The problem I guess is that the check doesn't seem to be detecting the correct thing in my case. Here's what I have in my theme, notice I'm merely trying to get rid of the background color:

`(markdown-code-face ((,class (:background unspecified))))
`(markdown-pre-face ((,class (:foreground unspecified))))
`(markdown-language-keyword-face ((,class (:weight bold))))
`(markdown-comment-face ((,class (:strike-through nil))))

Everything else does theme correctly. So my first suspicion was that maybe the theme was being loaded after markdown-mode, but I'm loading my theme (with a :demand t and a call to load-theme) very early on in my setup, with markdown-mode being deferred until a file is loaded with .md or .markdown.

One thing I did to test things further was load a markdown file, then observe that other markdown faces above are themed as I themed them, then eval-expression (get 'markdown-code-face 'saved-face) which yielded nil, then I figured I'd look at the symbol's entire property list with (symbol-plist 'markdown-code-face) (at least I think this makes sense) and I got:

(theme-face
 ((solarized-ext
   ((((class color)
      (min-colors 89))
     (:background unspecified)))))
 face-defface-spec
 ((t
   (:inherit fixed-pitch)))
 face-modified t face 436 face-documentation "Face for inline code, pre blocks, and fenced code blocks.")

Then I did a load-theme of my theme again and (get 'markdown-code-face 'saved-face) again yielded nil, and again (symbol-plist 'markdown-code-face) and I got:

(theme-face
 ((solarized-ext
   ((((class color)
      (min-colors 89))
     (:background unspecified)))))
 face-defface-spec
 ((t
   (:inherit fixed-pitch)))
 face-modified t face 436 face-documentation "Face for inline code, pre blocks, and fenced code blocks.")

So could it be that the check used isn't detecting the fact that in my case, it's styled? Or could this indeed point to an underlying issue in my configuration?

I could work around this by fseting markdown-update-code-face to ignore but I wanted to figure out what's really going on instead of sweeping things under the rug.

@jrblevin
Copy link
Owner

jrblevin commented Aug 29, 2017

Thanks for reporting this. I'm going to have to take a closer look, but haven't had time yet. The idea behind markdown-update-code-face is indeed to avoid changing the face if it has already been customized. Otherwise, the issue is to select a slightly lighter color when the background is dark or a slightly darker color when the background is light (I couldn't find a suitable face defined this way already).

Update: Another issue I was trying to solve is when the user changes themes from light to dark or vice-versa, but perhaps the new theme doesn't define markdown-code-face (and since it's new, most won't).

@jrblevin
Copy link
Owner

I was able to reproduce what you were seeing. The above commit should fix this, but let me know if not.

For now, I'll just set the background once at load time with defface, which will honor any theme changes.

If anyone knows of a way to adapt a face to always be lighter or darker than the default background by some fixed percentage without clobbering customizations, then I'd like to know about it.

@blaenk
Copy link
Author

blaenk commented Aug 29, 2017

I can confirm this worked for me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants