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

Fix non-graphical color switch #24

Merged
merged 1 commit into from Mar 9, 2017
Merged

Conversation

eigengrau
Copy link
Contributor

@eigengrau eigengrau commented Mar 7, 2017

(display-graphic-p) will evaluate to nil when starting Emacs in daemon mode.
Instead, we now use the attribute switch system provided by custom.el, which
apply definitions when a frame is created and will do the right thing when
running Emacs daemon.

Fixes #23

`(display-graphic-p)` will evaluate to nil when starting Emacs in daemon mode.
Instead, we now use the attribute switch system provided by custom.el, which
apply definitions when a frame is created and will do the right thing when
running Emacs daemon.

Fixes dracula#23
@eigengrau
Copy link
Contributor Author

Before
After

@@ -56,7 +55,8 @@
'dracula
;; default
`(cursor ((,class (:background ,fg3))))
`(default ((,class (:background ,bg1 :foreground ,fg1))))
`(default ((((type nil)) (:background "#000000" :foreground ,fg1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach of using type nil but documentation around default is kind of vague. Will having multiple defaults like this mean that if type is nil, then only the background of #000000 will be set, otherwise we'll use the standard bg1 (aka.. if/else)? Or will this apply both but type nil has a different default? I'm just trying to understand how this code is evaluated.

Copy link
Contributor Author

@eigengrau eigengrau Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my test runs, it’s a short-cut semantics; first match wins. So from my tests, the evaluation goes:

Frame shown on terminal

  • tries matching type nil against type type nil (the value representing terminal frames)
  • succeeds
  • applies settings
  • next condition (,class…) is not checked

Frame shown on non-terminal

  • tries matching type nil against type x, type w32, etc.
  • fails
  • next condition is checked and applied

I tested this by setting only the background in the first condition, and only the foreground in the second condition, and I only ever got one of the two applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, had to edit. The correct term would be short-cut semantics, not fall-through, which was the term I used accidentally, and which is exactly the opposite.

Copy link
Contributor Author

@eigengrau eigengrau Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silly me. It’s documented like that, too:

where DISPLAY is a form specifying conditions to match certain
terminals and ATTS is a property list (ATTR VALUE ATTR VALUE...)
specifying face attributes and values for frames on those
terminals.  On each terminal, the first element with a matching
DISPLAY specification takes effect, and the remaining elements in
SPEC are disregarded.

C-h f RET defface RET

Copy link
Contributor Author

@eigengrau eigengrau Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure I got your question about default though. The current frame being default applies to the invocation of (window-system) without arguments, which I suppose is what custom.el does, and it’s evaluated every time a new frame is created.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That answers my question, thanks! 🙂

I was just wondering how the code was evaluated. I was guessing it would short circuit, but wasn't sure. Thanks!

@film42
Copy link
Contributor

film42 commented Mar 8, 2017

:shipit:

I don't have a computer with me right now but I'll merge this in tomorrow morning. My phone is currently spazzing so I'll wait until I'm back at my laptop to merge.

@film42 film42 merged commit 0c7dd51 into dracula:master Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants