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

Adding dark title bar option for ns build. #21

Merged
merged 1 commit into from Mar 24, 2017

Conversation

Projects
None yet
2 participants
@jwintz
Contributor

jwintz commented Mar 24, 2017

This adds a dark styled title bar on mac and breaks no feature. Tested on Sierra (10.12.3).

screen shot 2017-03-24 at 17 42 55

@d12frosted d12frosted merged commit 6aad7ed into d12frosted:master Mar 24, 2017

@d12frosted

This comment has been minimized.

Show comment
Hide comment
@d12frosted

d12frosted Mar 24, 2017

Owner

Just nice! 💯

Do you know anything about borderless patch? 😸

Owner

d12frosted commented Mar 24, 2017

Just nice! 💯

Do you know anything about borderless patch? 😸

@d12frosted

This comment has been minimized.

Show comment
Hide comment
@d12frosted

d12frosted Mar 24, 2017

Owner

BTW, do you mind if I use your screenshot for README file?

Owner

d12frosted commented Mar 24, 2017

BTW, do you mind if I use your screenshot for README file?

@jwintz

This comment has been minimized.

Show comment
Hide comment
@jwintz

jwintz Mar 24, 2017

Contributor

Many thanks ! How long before it get available in brew ?

Please, be my guest, use the screenshot for the README !

I have not tried the borderless patch since in use either emacs quickly with a window, or for many weeks (at work typically) fullscreen, using MacOS great maximization as a space option. I do not quite understand the benefit of the borderless feature.

PS: I'm not using spacemacs, my config is here: https://github.com/jwintz/prolusion

Contributor

jwintz commented Mar 24, 2017

Many thanks ! How long before it get available in brew ?

Please, be my guest, use the screenshot for the README !

I have not tried the borderless patch since in use either emacs quickly with a window, or for many weeks (at work typically) fullscreen, using MacOS great maximization as a space option. I do not quite understand the benefit of the borderless feature.

PS: I'm not using spacemacs, my config is here: https://github.com/jwintz/prolusion

@jwintz

This comment has been minimized.

Show comment
Hide comment
@jwintz

jwintz Mar 24, 2017

Contributor

Hummm, after a brew update and a brew upgrade, brew info shows me the --with-dark-title-bar option. However, using brew reinstall emacs-plus --HEAD --with-dark-title-bar gives me the following.

jwintz@oniris ~/.emacs  (develop) $ brew reinstall emacs-plus --HEAD --with-dark-title-bar
==> Reinstalling d12frosted/emacs-plus/emacs-plus --with-dark-title-bar
==> Cloning https://github.com/emacs-mirror/emacs.git
Updating /Users/jwintz/Library/Caches/Homebrew/emacs-plus--git
==> Checking out branch master
==> Downloading https://gist.githubusercontent.com/aatxe/260261daf70865fbf1749095de9172c5/raw/214b50c624
Already downloaded: /Users/jwintz/Library/Caches/Homebrew/emacs-plus--patch-5af2587e986db70999d1a791fca58df027ccbabd75f45e4a2af1602c75511a8c.diff
==> Downloading https://gist.githubusercontent.com/jwintz/853f0075cf46770f5ab4f1dbf380ab11/raw/bc30bd2e9
################################################################################################# 100.0%
Warning: Cannot verify integrity of emacs-plus--patch-    https%3A%2F%2Fgist.githubusercontent.com%2Fjwintz%2F853f0075cf46770f5ab4f1dbf380ab11%2Fraw%2Fbc30bd2e9a7bf6873f3a3e301d0085bcbefb99b0%2Femacs_dark_title_bar.patch.patch
A checksum was not provided for this resource
For your reference the SHA256 is: 742f7275f3ada695e32735fa02edf91a2ae7b1fa87b7e5f5c6478dd591efa162
==> Patching
==> Applying patch-multicolor-font.diff
patching file src/macfont.m
Hunk #1 succeeded at 2360 (offset -13 lines).
==> Applying emacs_dark_title_bar.patch
patching file src/nsterm.m
patch unexpectedly ends in middle of line
Hunk #1 succeeded at 6744 with fuzz 1.

I will:

  • provide the suggested SHA1
  • fix the patch

Sorry for the inconvenience. Should have tried my formula with brew before submitting the PR.

Contributor

jwintz commented Mar 24, 2017

Hummm, after a brew update and a brew upgrade, brew info shows me the --with-dark-title-bar option. However, using brew reinstall emacs-plus --HEAD --with-dark-title-bar gives me the following.

jwintz@oniris ~/.emacs  (develop) $ brew reinstall emacs-plus --HEAD --with-dark-title-bar
==> Reinstalling d12frosted/emacs-plus/emacs-plus --with-dark-title-bar
==> Cloning https://github.com/emacs-mirror/emacs.git
Updating /Users/jwintz/Library/Caches/Homebrew/emacs-plus--git
==> Checking out branch master
==> Downloading https://gist.githubusercontent.com/aatxe/260261daf70865fbf1749095de9172c5/raw/214b50c624
Already downloaded: /Users/jwintz/Library/Caches/Homebrew/emacs-plus--patch-5af2587e986db70999d1a791fca58df027ccbabd75f45e4a2af1602c75511a8c.diff
==> Downloading https://gist.githubusercontent.com/jwintz/853f0075cf46770f5ab4f1dbf380ab11/raw/bc30bd2e9
################################################################################################# 100.0%
Warning: Cannot verify integrity of emacs-plus--patch-    https%3A%2F%2Fgist.githubusercontent.com%2Fjwintz%2F853f0075cf46770f5ab4f1dbf380ab11%2Fraw%2Fbc30bd2e9a7bf6873f3a3e301d0085bcbefb99b0%2Femacs_dark_title_bar.patch.patch
A checksum was not provided for this resource
For your reference the SHA256 is: 742f7275f3ada695e32735fa02edf91a2ae7b1fa87b7e5f5c6478dd591efa162
==> Patching
==> Applying patch-multicolor-font.diff
patching file src/macfont.m
Hunk #1 succeeded at 2360 (offset -13 lines).
==> Applying emacs_dark_title_bar.patch
patching file src/nsterm.m
patch unexpectedly ends in middle of line
Hunk #1 succeeded at 6744 with fuzz 1.

I will:

  • provide the suggested SHA1
  • fix the patch

Sorry for the inconvenience. Should have tried my formula with brew before submitting the PR.

@d12frosted

This comment has been minimized.

Show comment
Hide comment
@d12frosted

d12frosted Mar 24, 2017

Owner

Many thanks ! How long before it get available in brew ?

After I pushed you just need to brew update and that's all 😸 Now you can build with new option.

Please, be my guest, use the screenshot for the README !

Great, thank you!

I have not tried the borderless patch since in use either emacs quickly with a window, or for many weeks (at work typically) fullscreen, using MacOS great maximization as a space option. I do not quite understand the benefit of the borderless feature.

Ah I see. Well, I wondered if you can help making it available on head version 😸

provide the suggested SHA1

Oh right, I forgot about it. Following code should fix that.

  if build.with? "dark-title-bar"
    patch do
      url "https://gist.githubusercontent.com/jwintz/853f0075cf46770f5ab4f1dbf380ab11/raw/bc30bd2e9a7bf6873f3a3e301d0085bcbefb99b0/emacs_dark_title_bar.patch"
      sha256 "742f7275f3ada695e32735fa02edf91a2ae7b1fa87b7e5f5c6478dd591efa162"
    end
  end

Sorry for the inconvenience. Should have tried my formula with brew before submitting the PR.

No problem. Take your time 😸

And the funny thing - patch works for me. But provides transparent title bar instead of black one. And the funny thing is that title itself is written in white (I am not sure why it became grey when I made a screenshot), so I don't see it well. But I don't care much about it 😸

screenshot 2017-03-24 21 39 53

screenshot 2017-03-24 21 41 37

Owner

d12frosted commented Mar 24, 2017

Many thanks ! How long before it get available in brew ?

After I pushed you just need to brew update and that's all 😸 Now you can build with new option.

Please, be my guest, use the screenshot for the README !

Great, thank you!

I have not tried the borderless patch since in use either emacs quickly with a window, or for many weeks (at work typically) fullscreen, using MacOS great maximization as a space option. I do not quite understand the benefit of the borderless feature.

Ah I see. Well, I wondered if you can help making it available on head version 😸

provide the suggested SHA1

Oh right, I forgot about it. Following code should fix that.

  if build.with? "dark-title-bar"
    patch do
      url "https://gist.githubusercontent.com/jwintz/853f0075cf46770f5ab4f1dbf380ab11/raw/bc30bd2e9a7bf6873f3a3e301d0085bcbefb99b0/emacs_dark_title_bar.patch"
      sha256 "742f7275f3ada695e32735fa02edf91a2ae7b1fa87b7e5f5c6478dd591efa162"
    end
  end

Sorry for the inconvenience. Should have tried my formula with brew before submitting the PR.

No problem. Take your time 😸

And the funny thing - patch works for me. But provides transparent title bar instead of black one. And the funny thing is that title itself is written in white (I am not sure why it became grey when I made a screenshot), so I don't see it well. But I don't care much about it 😸

screenshot 2017-03-24 21 39 53

screenshot 2017-03-24 21 41 37

@jwintz

This comment has been minimized.

Show comment
Hide comment
@jwintz

jwintz Mar 24, 2017

Contributor

Well, that is an unexpected behaviour. Actually I first went for something like hard coding the background color value to black but I found out that the appearance stuff is more elegant. Point is, we do not really have control over what it does. But fortunately, according to your screenshots, the behaviour is quite good: light when the theme is light, dark when the theme is dark.

Contributor

jwintz commented Mar 24, 2017

Well, that is an unexpected behaviour. Actually I first went for something like hard coding the background color value to black but I found out that the appearance stuff is more elegant. Point is, we do not really have control over what it does. But fortunately, according to your screenshots, the behaviour is quite good: light when the theme is light, dark when the theme is dark.

@jwintz

This comment has been minimized.

Show comment
Hide comment
@jwintz

jwintz Mar 24, 2017

Contributor

From Apple's doc:

If the art for a specific view can’t be found, AppKit searches for the art in the appearances of the view’s ancestors. A nil appearance means that a view uses the default Aqua appearance; a non-nil appearance means that the view uses an ancestor’s appearance.

When AppKit draws a control, it automatically sets the current appearance on the current thread to the control’s appearance. The current appearance can influence the actual drawing path and the return values you get when you access system fonts and colors. The current appearance also affects the appearance of text and images, such as the text and template images that can be displayed in a toolbar.

That explains your screenshots and makes things way better than I imagined: it actually infers the title bar colour based on the ancestors ones. The option should then be called natural-title-bar and be set by default.

PS: Still trying to get this work through homebrew :-~

Contributor

jwintz commented Mar 24, 2017

From Apple's doc:

If the art for a specific view can’t be found, AppKit searches for the art in the appearances of the view’s ancestors. A nil appearance means that a view uses the default Aqua appearance; a non-nil appearance means that the view uses an ancestor’s appearance.

When AppKit draws a control, it automatically sets the current appearance on the current thread to the control’s appearance. The current appearance can influence the actual drawing path and the return values you get when you access system fonts and colors. The current appearance also affects the appearance of text and images, such as the text and template images that can be displayed in a toolbar.

That explains your screenshots and makes things way better than I imagined: it actually infers the title bar colour based on the ancestors ones. The option should then be called natural-title-bar and be set by default.

PS: Still trying to get this work through homebrew :-~

@d12frosted

This comment has been minimized.

Show comment
Hide comment
@d12frosted

d12frosted Mar 24, 2017

Owner

That explains your screenshots and makes things way better than I imagined: it actually infers the title bar colour based on the ancestors ones. The option should then be called natural-title-bar and be set by default.

Yeah, exactly what I wanted to propose 😸 If you feel like it - go forth and send another PR on name change 😸

As per making it on be default - I'll think about it. While I like this option some people might get frustrated 😸 But I'll make sure that many people are aware of this new option after we settle details 😸

Owner

d12frosted commented Mar 24, 2017

That explains your screenshots and makes things way better than I imagined: it actually infers the title bar colour based on the ancestors ones. The option should then be called natural-title-bar and be set by default.

Yeah, exactly what I wanted to propose 😸 If you feel like it - go forth and send another PR on name change 😸

As per making it on be default - I'll think about it. While I like this option some people might get frustrated 😸 But I'll make sure that many people are aware of this new option after we settle details 😸

@jwintz

This comment has been minimized.

Show comment
Hide comment
@jwintz

jwintz Mar 24, 2017

Contributor

Ok ;-) let's switch to #23

Contributor

jwintz commented Mar 24, 2017

Ok ;-) let's switch to #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment