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

Tweak mode-line faces #174

Closed
wants to merge 1 commit into from

Conversation

ryanartecona
Copy link
Contributor

This PR

  1. removes :box from mode-line faces
  2. adds mode-line-inactive overline/underline colors

The reason for (1) is that the :box property isn't applied the same way to xpm images or non-text spaces the same way as it is applied to normal text within the mode-line. Or something like that. This resulted in weird underline/overline discontinuities in the presence of xpm images or other non-text stuff in the mode-line when high-contrast-mode-line was nil (see below). Powerline uses xpm images as separators.

Appropriate underline/overline properties are now relied upon exclusively, and IIUC, were responsible for the lines seen before this PR anyway in normal (text) mode-lines. To my knowledge, this shouldn't affect normal mode-lines.

The reason for (2) is to specify appropriate underlines/overlines in support of (1). Colors shouldn't change except that the inactive mode-line now gets an overline of s-line and an underline of base01 when high-contrast-mode-line is t, where before the overline and underline would both be base02 in that case, and difficult to see (see below).

Before:

  • with high contrast: screen shot 2015-04-01 at 12 02 49 pm
  • without high contrast: screen shot 2015-04-01 at 12 03 35 pm

After:

  • with high contrast: screen shot 2015-04-01 at 11 59 11 am
  • without high contrast: screen shot 2015-04-01 at 12 00 28 pm

  - remove :box from mode-line faces
  - add mode-line-inactive overline/underline
@thomasf
Copy link
Collaborator

thomasf commented Apr 1, 2015

I'll have a look at this soon, I'll have to verify it with a couple of more things before merging.

@ryanartecona
Copy link
Contributor Author

In case it helps focus on things, what most concerned me were (a) whether removing :box breaks the face in certain environments that depend on it (I am new to emacs and only use it in a standalone Mac app), and (b) the header-line face, which I didn't touch, also uses a :box but with a line-width of 2, so I was unsure if/how my changes shpould be translated to that face. I also don't use a header-line (does anyone?).

I tested neither (a) nor (b).

On Apr 1, 2015, at 3:57 PM, Thomas Frössman notifications@github.com wrote:

I'll have a look at this soon, I'll have to verify it with a couple of more things before merging.


Reply to this email directly or view it on GitHub.

@c256
Copy link

c256 commented Apr 1, 2015

Header-lines are used by a few modes, including Info, ruler, some tabbed-buffer emulation, and eww. I'm willing to bet that you actually do use a header line now and then and just don't notice it.

Have you tried your change with emacs in a terminal?

@thomasf
Copy link
Collaborator

thomasf commented Apr 1, 2015

IIRC I had to do something specific for mode/header lines to work well together.. I think It has something todo with avoiding double lines between the header/mode or something.. Not saying that this patch breaks anything, just adding information for the moment...

Here's how the header/mode lines looks without this patch:

image

@ryanartecona
Copy link
Contributor Author

@c256 re header-line: You're probably right. Do you happen to know if any of those try putting images in the header-line? If so, I can try to install if necessary and try it out; if not, it may be ok to leave the header-line face as-is as far as this PR is concerned.

re non-gui emacs: I just tried it in iTerm with xterm-256colors on, both before and after this PR's changes. It looks unaffected to me—the mode-line just gets a textual underline and no overline instead of graphical underline/overline.

@ryanartecona
Copy link
Contributor Author

@c256 scratch that, I didn't do it properly before. This change adds an underline to both active and inactive mode-line faces, where they had no underline before, when high-contrast-mode-line is on; it does not affect the mode-line faces without high-contrast on; they had textual underlines already, and still do.

Should the mode-line underline only be set if emacs is loaded in a GUI environment? And if so, what function/variable reports that, so I can update the PR?

@ryanartecona
Copy link
Contributor Author

Any update on this? Anyone care whether an underline gets added to active/inactive high-contrast modelines in a terminal? Any related changed need to happen on the header line?

@thomasf
Copy link
Collaborator

thomasf commented Apr 10, 2015

I never got to this becase of all the other smalle *-line issues that popped up and it seemd that it was for the best to resolve those first.. I will try to include to check the things I want to and merge this the next time I work on solarized.

@ryanartecona
Copy link
Contributor Author

@thomasf works for me! I agree with you. No rush here, just didn't want it to be forgotten :)

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