sellout / emacs-color-theme-solarized

Emacs highlighting using Ethan Schoonover’s Solarized color scheme
http://ethanschoonover.com/solarized
MIT License
1.15k stars 201 forks source link

Better handling of emacsclient, and better variable names #17

Closed ahyatt closed 9 years ago

ahyatt commented 13 years ago

One of the problems with the current solarized theme for emacs 24 is that it doesn't support emacsclient. To properly support emacsclient, we need to build the colors for terminals and degraded terminals into the facespecs themselves, so that's what I did.

Along the way, I had to make variables to hold the values of the colors, so I gave the variables base00, etc, more meaningful names. These are a starting point, and I'm not quite happy with them, but I think it's a step forward.

sellout commented 13 years ago

It's very possible that I misunderstood your comment, and if so I apologize and hopefully you can help me see what I'm missing. Barring that, here's what I think:

I'm not sure how emacsclient is related to Solarized at all. Emacsclient just sends files to an already running emacs. It shouldn't effect how that emacs instance is styled at all. Now, I think that if emacsclient doesn't see a running emacs server, it'll start a new emacs instance itself in the terminal. But in that case, it falls on the terminal to support Solarized – if you're not using Solarized in your term, and your term doesn't support 256 colors, then loading Solarized themes in a terminal emacs is not going to look good. In this case, you should conditionalize the solarized theme to only load when you are using a GUI.

I appreciate that you are having a problem with getting the colors to work correctly, but I really need to see screenshots and maybe a step-by-step of what is happening to figure out the underlying problem. Also, any of the following would help: the platform you're on, the name of the terminal emulator that you're using, and the number of colors supported by your term.

As for the color names, I too am not particularly enamored with them. However, it's an issue that should be taken up with Solarized proper. It's important that the names are consistent to make it easier to have similar themes across various editors and other tools. Changing them for Emacs only is not an option.

ahyatt commented 13 years ago

The color names itself aren't that important, and I can change them back. Emacsclient does necessitate a change in color theme definitions. With emacsclient, you have the possibility that two clients are connected to the same emacs, which is using just one color theme. If you want those two clients types to display different colors, you have to define different colors for those different clients.

In my specific instance, I was starting an emacs in x-windows, and then starting another one in terminal later. Your solarized package for emacs 24 does have some mechanism for adjusting the colors for terminals, but that's on theme definition time, not on client connect time. So, on connecting with terminal, I was not getting the terminal-specific colors, and the colors were in fact unusable in my particular client.

By changing the fontspecs to handle terminals, the right thing happens on connect - terminals get the terminal colors.

On Sat, May 21, 2011 at 11:40 PM, sellout reply@reply.github.com wrote:

It's very possible that I misunderstood your comment, and if so I apologize and hopefully you can help me see what I'm missing. Barring that, here's what I think:

I'm not sure how emacsclient is related to Solarized at all. Emacsclient just sends files to an already running emacs. It shouldn't effect how that emacs instance is styled at all. Now, I think that if emacsclient doesn't see a running emacs server, it'll start a new emacs instance itself in the terminal. But in that case, it falls on the terminal to support Solarized – if you're not using Solarized in your term, and your term doesn't support 256 colors, then loading Solarized themes in a terminal emacs is not going to look good. In this case, you should conditionalize the solarized theme to only load when you are using a GUI.

I appreciate that you are having a problem with getting the colors to work correctly, but I really need to see screenshots and maybe a step-by-step of what is happening to figure out the underlying problem. Also, any of the following would help: the platform you're on, the name of the terminal emulator that you're using, and the number of colors supported by your term.

As for the color names, I too am not particularly enamored with them. However, it's an issue that should be taken up with Solarized proper. It's important that the names are consistent to make it easier to have similar themes across various editors and other tools. Changing them for Emacs only is not an option.

Reply to this email directly or view it on GitHub: https://github.com/sellout/emacs-color-theme-solarized/pull/17#issuecomment-1216258

perusio commented 13 years ago

I had this same issue with emacsclient in emacs 23.3.1 ahyatt pull request code fixes the issue.

OTOH the cursor color setting code gets messed up seriously. So I ended up with an hack like:

(global-set-key (kbd "M-s-t") 'color-theme-solarized-dark)

I just press the above keychord when the first frame starts et voilá. Ça marche.

sellout commented 12 years ago

Sorry this has sat for so long.

@ahyatt – if you could submit a new pull request with only the emacsclient changes, I think I can merge that.

ahyatt commented 12 years ago

This pull request is just for the changes to get emacsclient to work. It's a lot of refactoring to handle this case. It's been a while since I've looked at this, but is there anything in my commit that you think shouldn't be in there? The color names now have a semantic meaning since they have a different value for light and dark themes, so they represent a concept now, not a color. So, if you really want to revert to the old color names, I'll have to rethink how I did this change and see if there's a better way.

sellout commented 12 years ago

The problem with the colors is that they aren't really ours to change. Keeping the names the same as are used by the other modules (VIM, etc.) makes it easier to see where differences have crept in between them and reconcile those. Also, your color names are a bit wrong (e.g., background-suble and background-grey are actually foreground colors).

I understand if you don't want to have to get your head back into this nine months after you sorted it out. I'm happy to go replace the old color names back in and then see which diffs remain to be merged.

ahyatt commented 12 years ago

OK, I think I can see a way I can fix things up so that the variable names are kept the same way. Let me work on refining my patch, I should have something for you in the next few days.

On Sun, Feb 12, 2012 at 10:33 AM, Greg Pfeil reply@reply.github.com wrote:

The problem with the colors is that they aren't really ours to change. Keeping the names the same as are used by the other modules (VIM, etc.) makes it easier to see where differences have crept in between them and reconcile those. Also, your color names are a bit wrong (e.g., background-suble and background-grey are actually foreground colors).

I understand if you don't want to have to get your head back into this nine months after you sorted it out. I'm happy to go replace the old color names back in and then see which diffs remain to be merged.


Reply to this email directly or view it on GitHub: https://github.com/sellout/emacs-color-theme-solarized/pull/17#issuecomment-3928446

ahyatt commented 12 years ago

OK, please check out the new version and let me know what you think.

sellout commented 11 years ago

You guys have probably left Solarized behind long ago, but please try the “unified” branch, which has only a “solarized” theme, letting background-mode determine whether to use dark or light for a particular frame.

The reason it’s in a separate branch is that I don’t think it currently works with color-theme (which is necessary before Emacs 24).

expez commented 11 years ago

Could you merge the two branches in some way? I get that you want to make this work for emacs versions < 24, but tons of people would benefit from these changes if they were available in the package manager and could be activated easily. I'm probably more interested than most in this, but I really don't want to clone the repo, vendor the files, and then check back every now and again to pull in new changes.

sellout commented 9 years ago

The unified branch has now been merged.