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 mode line suggested in README resulting in unreadable mode line #129

Closed
rudolf-adamkovic opened this issue May 5, 2021 · 3 comments
Closed

Comments

@rudolf-adamkovic
Copy link
Contributor

rudolf-adamkovic commented May 5, 2021

In README, we say:

You can change the entire mode line's foreground and background to indicate whether God mode is active as follows: [emphasis mine]

Then, the sample code below changes only the background color. As a result, with default colors (or any other light color theme), the mode line becomes unreadable, rendering black text on black background. We must change both the background and the foreground color.

For reference, below is my my-god-mode-update-mode-line. This is tied to modus-themes, but we could pick some "raw" colors that look good.

(defun my-god-mode-update-mode-line ()
  (modus-themes-with-colors
    (cond
     (god-local-mode
      (set-face-attribute 'mode-line nil
                          :foreground yellow-refine-fg
                          :background yellow-refine-bg
                          :box yellow)
      (set-face-attribute 'mode-line-inactive nil
                          :foreground yellow-nuanced-fg
                          :background yellow-nuanced-bg
                          :box yellow))
     (t
      (set-face-attribute 'mode-line nil
			  :foreground fg-active
			  :background bg-active
			  :box fg-alt)
      (set-face-attribute 'mode-line-inactive nil
			  :foreground fg-inactive
			  :background bg-inactive
			  :box bg-region)))))

Note: I can do this later.

@darth10 darth10 closed this as completed May 5, 2021
@darth10 darth10 reopened this May 5, 2021
@darth10
Copy link
Collaborator

darth10 commented May 5, 2021

Good spotting, @salutis!

I (over)simplified the function in the README and came up with this:

(defun my-god-mode-update-mode-line ()
  (cond (god-local-mode
         (progn
           (set-face-attribute 'mode-line nil
                               :foreground "#e9e2cb"
                               :background "#0a2832")
           (set-face-attribute 'mode-line-inactive nil
                               :foreground "#e9e2cb"
                               :background "#0a2832")))
        (t
         (progn
           (set-face-attribute 'mode-line nil
                               :foreground "#0a2832"
                               :background "#e9e2cb")
           (set-face-attribute 'mode-line-inactive nil
                               :foreground "#0a2832"
                               :background "#e9e2cb")))))

I feel users can add in other attributes like :box as they prefer.
Happy to add it in if needed as well.
Keen to hear your thoughts 😄

@rudolf-adamkovic
Copy link
Contributor Author

@darth10

I think we should use different colors for active and inactive mode lines. Setting both states to the same color sets a bad example and decreases accessibility of Emacs for no good reason.

Here is a version with properly differentiated active/inactive state:

(defun my-god-mode-update-mode-line ()
  (cond
   (god-local-mode
    (set-face-attribute 'mode-line nil
                        :foreground "#604000"
                        :background "#fff29a")
    (set-face-attribute 'mode-line-inactive nil
                        :foreground "#3f3000"
                        :background "#fff3da"))
   (t
    (set-face-attribute 'mode-line nil
			:foreground "#0a0a0a"
			:background "#d7d7d7")
    (set-face-attribute 'mode-line-inactive nil
			:foreground "#404148"
			:background "#efefef"))))

Feel free to swap the colors; the ones above are from Modus Themes.

P.S. progn is not necessary, as explained to me by Protesilaos Stavrou here:

https://gitlab.com/protesilaos/modus-themes/-/issues/187

@darth10
Copy link
Collaborator

darth10 commented May 5, 2021

Setting both states to the same color sets a bad example and decreases accessibility of Emacs for no good reason.

Couldn't agree more 👌

progn is not necessary

Good catch!
This function has been in the README for quite a while and I've been modifying it without really paying attention to that.

It looks like you've picked up a decent amount of Emacs Lisp in a relatively short amount of time 😄
Just like you, I have the awesome Emacs Lisp community to thank for whatever I know!

I'll update the README shortly.

@darth10 darth10 closed this as completed in 15338d9 May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants