bbatsov / solarized-emacs

The Solarized colour theme, ported to Emacs.
775 stars 176 forks source link

Version FUTURE #354

Open thomasf opened 4 years ago

thomasf commented 4 years ago

Are there limitations, annoyances, inconsistencies or just something that's inconvenient about the theme right now?

Most important points

Please comment here if you have any wishes of what you want to be able to do with the theme that you cant do now or if something is bothers you.

The end result of this discussion not be any new version at all, just insight.

(note: I have minimized a lot of comments because they are off topic. This issue is not for endless complaints about past decisions, it's about looking forward. I kept a little bit of the least off topic parts visible for context.)

thomasf commented 4 years ago

A small list of things that comes to mind right now.

To keep

Stay Solarized

I do NOT want to get away from the basic solarized "limitation" of 8 tones and 8 accent colors. This is the core values of solarized in general and this theme in particular. Even if we allow for extending the palette some times to make up for Emacs lack of blending we also have a much stricter usage of some accent colors than most other Solarized themes has.

Solarized is a 24bit color scheme

I believe that it's time to officially state that this is a 24bit theme, in practical terms this is already more or less true but we still have the oldest still open issue "Incorrect Colours in Terminal" hanging around. Almost all terminal emulators that people use will probably have 24bit support within a few years anyways.

To improve

Composability at lower levels

I want it to be possible to have snippets of face definitions and palettes that splices together a finished theme. Instead of filling the insides of the theme body with conditionals I would like to be able to compose a theme from lists, something like (create-solarized-theme :faces solarized-faces-base solarized-faces-more-italics solarized-faces-high-contrast-modeline :palettes solarized-palette-dark my-high-contrast-blue ) We are in semi constant battle with the theme system when doing things like this but maybe there is a clear way forward somewhere.

To remove

solarized-theme.el

All the themes are in the same directory as a file named solarized-theme.el, this file is picked up by load-theme but fails to load. That file should probably not exist.

accent colors with -l -d -hc -lc suffixes

The usage of these are note well defined and were intended to be used both with a light and dark solarized theme. That was proven to not really work and now they just live in a unknown space. I have begun a project to either get rid of the internal usage or clearly define their purpose. Even if the theme itself stops using them they should be kept around if someone else is referencing them.

conao3 commented 4 years ago

Solarized-theme should offer only solarized-dark and solarized-light. The ability to create other themes is a minor thing. Also, those who use the extras are highly familiar with solarized internals, and specifically no one will enable the bundled solarized-wombat-dark-theme.

Downloading wasted code from MELPA and byte-compiling is a waste of computer resources. In Version 2.0, please remove the following files from the package and only give me what I need.

(user can use high-contrast theme with some options for solarize-theme?) Or, if you want a different bundle theme like doom-theme, you will be welcomed to create a new package with a package name like solarized-misc-themes and place the file there.

thomasf commented 4 years ago

Solarized-theme should offer only solarized-dark and solarized-light. The ability to create other themes is a minor thing.

While I respect your opinion that not everything needs to be included in one package I would like to point out that it's still the same theme, just with different palettes. All the work that has gone into theming using the Solarized palette is reusable with other palettes as long as they are able to fit within the 8 tones 8 accents formula.

I'm pretty sure that more computer resources will be consumed by separating into multiple packages but that's hardly relevant.

conao3 commented 4 years ago

I would like to point out that it's still the same theme, just with different palettes.

So you should not bundle those files. How to use it is to write in README. If you want to prepare a concrete example, you should place the file in another directory that is not packaged by MELPA. Anyway, my opinion is over because you want to ask for opinions widely in this thread.

alphapapa commented 4 years ago

A small list of things that comes to mind right now.

  1. All the themes are in the same directory as a file named solarized-theme.el, this file is picked up by load-theme but fails to load. That file should probably not exist.
  2. I believe that it's time to officially state that this is a 24bit theme, in practical terms this is already more or less true but we still have the oldest still open issue "Incorrect Colours in Terminal"
  3. I want it to be possible to have snippets of face definitions and palettes that splices together a finished theme. Instead of filling the insides of the theme body with conditionals I would like to be able to compose a theme from lists, something like (create-solarized-theme :faces solarized-faces-base solarized-faces-more-italics solarized-faces-high-contrast-modeline :palettes solarized-palette-dark my-high-contrast-blue ) We are in semi constant battle with the theme system when doing things like this but maybe there is a clear way forward somewhere.
  4. I do NOT want to get away from the basic solarized "limitation" of the 8 tones and 8 accent colors. This is the core values of solarized even if we allow for extending the palette some times to make up for Emacs lack of blending support.

Those are some very interesting ideas regarding making themes easier to define. A library to define Emacs themes more easily could be very helpful.

But please do those in a separate project which is dedicated to defining Emacs themes in a flexible, powerful way. This project and repo is for Solarized themes, and it should not be repurposed into a general-purpose theme framework.

After you're created such a project and it has become mature, this repo and its Solarized themes could be converted to use that framework.

alphapapa commented 4 years ago

While I respect your opinion that not everything needs to be included in one package I would like to point out that it's still the same theme, just with different palettes. All the work that has gone into theming using the Solarized palette is reusable with other palettes as long as they are able to fit within the 8 tones 8 accents formula.

No, Solarized is its palette. Its own web site says so:

Solarized is a sixteen color palette (eight monotones, eight accent colors) designed for use with terminal and gui applications. It has several unique properties. I designed this colorscheme with both precise CIELAB lightness relationships and a refined set of hues based on fixed color wheel relationships. It has been tested extensively in real world use on color calibrated displays (as well as uncalibrated/intentionally miscalibrated displays) and in a variety of lighting conditions.

The generic 8-hues-8-accents formula is not Solarized, and other such themes that follow that generic pattern are not Solarized and do not belong in solarized-emacs.

thomasf commented 4 years ago

But please do those in a separate project which is dedicated to defining Emacs themes in a flexible, powerful way. This project and repo is for Solarized themes, and it should not be repurposed into a general-purpose theme framework.

I haven't created anything yet, these are just ideas and/or wishes for now. Please add your own if you have any. I have no idea what this issue will lead to.

alphapapa commented 4 years ago

I have no idea what this issue will lead to.

Then that is the first problem to be solved: to answer the question, What is the purpose of the solarized-emacs package?

AFAIK it is to provide Solarized themes for Emacs. I think that should continue to be its purpose. If you want to write a theme library, wonderful, that would be very helpful to theme authors. Please do so in the appropriate place, and maybe these Solarized themes can use it someday.

@bbatsov May I ask, are you still running this project? If so, it seems necessary now to define its purpose before it may be reconfigured into a theme-definition framework which happens to include some Solarized themes. If that is going to happen, it may be necessary to fork the project to preserve its usefulness as-is.

thomasf commented 4 years ago

The generic 8-hues-8-accents formula is not Solarized, and other such themes that follow that generic pattern are not Solarized and do not belong in solarized-emacs.

No, but they are compatible with a Solarized theme if the colors have reasonably similar properties. They are also subsets and modified to fit this theme.

I have maybe spent several hundred hours adding and tweaking faces for this theme, I probably know more than anyone else about how well fitted it is for various palettes.

alphapapa commented 4 years ago

The generic 8-hues-8-accents formula is not Solarized, and other such themes that follow that generic pattern are not Solarized and do not belong in solarized-emacs.

No, but they are compatible with a Solarized theme if the colors have reasonably similar properties. They are also subsets and modified to fit this theme.

No, those are not Solarized, so they shouldn't be called Solarized.

I have maybe spent several hundred hours adding and tweaking faces for this theme, I probably know more than anyone else about how well fitted it is for various palettes.

Please respond to the points I have made. If you want to create variations on Solarized, wonderful, I might be interested in using them myself. If you want to make a theme-defining library, wonderful, I'll probably use it myself. But please do those in the appropriate place, not by commandeering this package.

thomasf commented 4 years ago

Then that is the first problem to be solved: to answer the question, What is the purpose of the solarized-emacs package?

The primary goal will always continue be that M-x load-theme solarized-dark/light will give you the best possible experience regardless of how the underlying code is organized. Palettes other than Solarized will never have influence over theming decisions , they are complementary.

thomasf commented 4 years ago

I do not expect non-solarized-palette Emacs themes existing in this repository to contain more than a list of color codes (a palette) and maybe in rare exceptions a handful of face adjustments for very basic faces.

alphapapa commented 4 years ago

You haven't answered any points that @conao3 or I have raised. You've simply ignored and dismissed our objections.

One of the things I, and many other users, value most about Emacs and its packages is stability. You and/or @conao3 have already broken my config with your recent change to the solarized-with-color-variables macro's signature. Now you're proposing to essentially remake the package from scratch into a theme-defining library which happens to include some Solarized themes, only some of which are actually Solarized themes.

Move fast and break things is not a motto that this project should follow. There's already enough of that in the software world.

I guess I should fork the project and reset HEAD to https://github.com/bbatsov/solarized-emacs/commit/55cd77b61b6968048c61e13358ba487d217f24c0. I'll rename it to ya-solarized or something like that.

thomasf commented 4 years ago

Now you're proposing to essentially remake the package from scratch into a theme-defining library which happens to include.

No, I wrote clearly that we should list wishes, wishes are not the same thing as requirements. I have updated the issue text to reflect this even more clearly now.

Move fast and break things is not a motto that this project should follow. There's already enough of that in the software world.

Exactly, I saw that happen and as a consequence I created this issue to collect ideas and and break it once instead of incrementally fixing things and breaking it multiple times.

Going to a breaking 2.0 after around 10 years is not moving fast and breaking things.

I guess I should fork the project and reset HEAD to 55cd77b. I'll rename it to ya-solarized or something like that.

If you think that that is less work than changing one argument to a function call in your own code (or whatever the breakage was) I wish you good luck. I have already spent around 30hrs since this weekend on core theme concerns, mostly picking out the diff/refine blends.

alphapapa commented 4 years ago

This is the problem:

(or whatever the breakage was)

You don't seem to care about, to use Linus's words, "breaking userspace." You want to do big things--that's wonderful, there are many improvements to be made. The problem is that you want to do them by breaking things here instead of in your own projects. That's not cool.

thomasf commented 4 years ago

This is the problem:

(or whatever the breakage was)

You don't seem to care about, to use Linus's words, "breaking userspace." You want to do big things--that's wonderful, there are many improvements to be made. The problem is that you want to do them by breaking things here instead of in your own projects. That's not cool.

I just forgot exactly what the breaking issue was. You do know it wasn't me who broke it, right?

We can extend that line of reasoning in absurdum as well if you like, every change of theming a face could be a breaking change for someone because in the end it's about taste as well as usability.

Now it just seems like you are after drama, you are cutting out one small part of my statement that included a very clear message about not wanting to break stuff to complain that I am breaking stuff.

Also, this is not the Linux kernel, we don't have millions of lines of code and billions of users. If we break something, even restructure all of the code people will still probably be able to adapt their customization in less than 5 minutes.

conao3 commented 4 years ago

I was mentioned. @alphapapa, you can't pursue its selfishness just because solarized internal function broke. I know that this is a violation of manners, but it is a bad joke that you are a member of helm and insist on it. Any Emacs user knows that helm does destruction and creation.

And I have repeatedly insisted that users can only use theme files and undocumented solarized-create-theme, all others are internal functions. I admit that the naming is wrong, but it is not desirable to rely heavily on internal functions just because Elisp functions are defined globally.

Also, I don't think it's a better choice to keep things broken as you wrote in the comment. https://github.com/alphapapa/prism.el/blob/49c8b97b286c7403bf239645964416200ea82dd5/prism.el#L774-L778

alphapapa commented 4 years ago

You merged the PR that broke the macro's signature. As a maintainer, it's your job to consider such breakage and avoid it when possible.

What I'm after is serving users' interests. Maintaining a widely used package is a matter of stewardship, which often means putting the users' interests before the maintainer's interests, sometimes at the expense of the maintainer's fun.

Of course, you could easily make a new repo and do the fun stuff there. Make your new thing, make it amazing, and then use it as a dependency here, when it's mature. But you seem unwilling to consider doing that for some reason.

If we break something, even restructure all of the code people will still probably be able to adapt their customization in less than 5 minutes.

5 minutes 10,000 users = 34 man-days of time. That's disrespectful of users unless the benefits are really* worth it.

Well, it's your project, so I'll leave you to it. I do appreciate your work on it, I just wish you'd do it separately, without breaking things.

alphapapa commented 4 years ago

I was mentioned. @alphapapa, you can't pursue its selfishness just because solarized internal function broke. I know that this is a violation of manners, but it is a bad joke that you are a member of helm and insist on it. Any Emacs user knows that helm does destruction and creation.

Before you blame me for breakage in Helm repositories, you should look at the commits I have actually made to Helm repos, which are very few and haven't broken anything, as well as discussions I've participated in, in which I have argued against breaking changes and gotten them reverted. I am consistent in this philosophy, and it's wrong of you to accuse me otherwise.

And I have repeatedly insisted that users can only use theme files and undocumented solarized-create-theme, all others are internal functions. I admit that the naming is wrong, but it is not desirable to rely heavily on internal functions just because Elisp functions are defined globally.

As you know, private/internal functions are merely a convention, as Emacs is in fact a Lisp system image. Users use code that is available. It's often poor stewardship to fall back on "that was a private function, you shouldn't have used it" to excuse breakage. It is, of course, a matter of judgment in each case.

Also, I don't think it's a better choice to keep things broken as you wrote in the comment. https://github.com/alphapapa/prism.el/blob/49c8b97b286c7403bf239645964416200ea82dd5/prism.el#L774-L778

I don't think you understand that code or that comment. If you wish to talk about it, you may comment on that repo, which its readme says is in "Early prototype stages."

In any case, it's bizarre for you to dig through my code in other, unrelated projects for something you think is ugly and try to use it as an insult here. I've never seen anyone do such a thing. I point out breakage you caused in this project, which you may or may not have thought was justified, which is a matter of judgment, and you respond by making false accusations of me about other projects?

Rest assured, you've certainly dissuaded me from participating further here.

thomasf commented 4 years ago

Of course, you could easily make a new repo and do the fun stuff there. Make your new thing, make it amazing, and then use it as a dependency here, when it's mature. But you seem unwilling to consider doing that for some reason.

Again, I am not ruling out anything at this point. I primarily want to know what people wishes . The low level theme construction stuff is absolutely to make the theme itself better. It doesn't have to be much more code than it is right now. As long that it is the right code it can probably be quire short but that's details about something that just exists as a wish right now. I do not have a clearly formulated picture in my head how it would work either.

Right now it's very cumbersome to give users more choices by means of defcustoms of a theme because of the (if (eq variant 'light..) and stuff spread out all over the several thousand lines long list of faces. There is a clear limit on how many customisations even can be added before it gets too hard to follow and test the permutations when working on faces. If these were broken out to some kind of composable units we can give users much more options.

As long as they are distinct entities we can have 8 different mode line looks and feels, right now a single defcustom causes this mess.

  (s-mode-line-fg (if solarized-high-contrast-mode-line
                              base03 base0))
          (s-mode-line-bg (if solarized-high-contrast-mode-line
                              base0 base02))
          (s-mode-line-underline (if solarized-high-contrast-mode-line
                                     nil s-line))

          (s-mode-line-buffer-id-fg (if solarized-high-contrast-mode-line
                                        'unspecified base1))
          (s-mode-line-inactive-fg (if solarized-high-contrast-mode-line
                                       base0 base01))
          (s-mode-line-inactive-bg (if solarized-high-contrast-mode-line
                                       base02 base03))
          (s-mode-line-inactive-bc (if solarized-high-contrast-mode-line
                                       base02 base02))

... somewhere else ...

    `(mode-line
         ((,class (:inverse-video unspecified
                                  :overline ,s-mode-line-bg
                                  :underline ,s-mode-line-underline
                                  :foreground ,s-mode-line-fg
                                  :background ,s-mode-line-bg
                                  :box (:line-width 1 :color ,s-mode-line-bg
                                                    :style unspecified)))))
       `(mode-line-buffer-id ((,class (:foreground ,s-mode-line-buffer-id-fg :weight bold))))
       `(mode-line-inactive
         ((,class (:inverse-video unspecified
                                  :overline ,s-mode-line-inactive-bc
                                  :underline ,s-mode-line-underline
                                  :foreground ,s-mode-line-inactive-fg
                                  :background ,s-mode-line-inactive-bg
                                  :box (:line-width 1 :color ,s-mode-line-inactive-bg
                                                    :style unspecified)))))

With composition of faces lists these could just simply be two different face definition blocks.

I have IIRC declined suggestions to make some things into defcustoms just to keep the code readable as it is constructed now, these are not problems I'm just making up. Some things can probably be retrofitted without breaking anything but others can't.

There is no requirement that a wish here has to break something

conao3 commented 4 years ago

?? @alphapapa, you mentioned prism.el at first at https://github.com/bbatsov/solarized-emacs/pull/330#discussion_r342851332 . So I read the code and found the comment.

It is a nice situation to have maintainers moving their hands. If there are no maintainers to move the hand, the package will be devastated and no one can use it. If you want to rely on broken solarized at some point, you should use Cask to specify the dependent commit and clean up the dependencies. Mentioning helm is my bad joke. I am sorry.

alphapapa commented 4 years ago

?? @alphapapa, you mentioned prism.el at first at #330 (comment) . So I read the code and found the comment.

Then, as I said, I don't think you understand the code or the comment--or, I don't understand what point you're trying to make about it.

It is a nice situation to have maintainers moving their hands. If there are no maintainers to move the hand, the package will be devastated and no one can use it.

Sure.

If you want to rely on broken solarized at some point, you should use Cask to specify the dependent commit and clean up the dependencies.

This package is not broken. I'm asking that it not be broken from users' perspectives.

Mentioning helm is my bad joke. I am sorry.

Ok, thanks.

conao3 commented 4 years ago

Maintaining code that the maintainer cannot maintain, even if it wasn't broken from the user's point of view, is not a good choice.

If you're a programmer, don't give such a low opinion. If it breaks temporarily from the user's point of view, changing it to a code that can be maintained is beneficial in the long run. You can't miss this advantage.

alphapapa commented 4 years ago

Yes, that's what programmers always say to justify rewriting working code. And the users are always the ones who pay the price. ;)

Sometimes it is actually justified.

In this case, the question seems to be about supporting additional, non-Solarized themes in this solarized-emacs package which already works. So is it really justified to rewrite everything?

Instead, how about making a new project called thomasf-conao3-solarizy-themes that does all the things better?

conao3 commented 4 years ago

If we break something, even restructure all of the code people will still probably be able to adapt their customization in less than 5 minutes.

5 minutes * 10,000 users = 34 man-days of time. That's disrespectful of users unless the benefits are really worth it.

My opinion. This formula is wrong. Only a few use internal functions like solarized-with-color-variables, and most users use solarized-theme via load-theme. We (@thomasf and me) have no intention of changing this usage or behavior. The impact related change is minimal or nothing.

thomasf commented 4 years ago

In this case, the question seems to be about supporting additional, non-Solarized themes in this solarized-emacs package which already works. So is it really justified to rewrite everything?

That is not the reason (hint: we already have support for that) I don't understand why you still say things like that after I repeatedly have explained and clarified the opposite

alphapapa commented 4 years ago

That is not the reason (hint: we already have that) , I don't understand why you still say things like that after I repeatedly have explained and clarified the opposite

Is this really hinging on the question of what qualifies as a "Solarized" theme?

thomasf commented 4 years ago

No. Even if the other palettes and their -theme.el files were removed exactly the same things would be needed. This recent multiple palette support just made it possible for me to start work on having one palette for Solarized dark and one for light in an convenient way. I have wanted it for some years now to be able to easily add full light/dark precalculated palettes to be able to achieve better mode specific blending. This work also just happened to create the possibility of adding in other palettes as well.

All this code was written to generate solarized colors, as a "bonus" it was very easy to create more "Solarized" using other colors but that was never the primary intention. Having more palettes in there has proven to be valuable when evaluating the color values because I can switch between the solarized's and other ones to get a better understanding what works and why.

https://github.com/bbatsov/solarized-emacs/blob/master/colorlab/main.go#L737

alphapapa commented 4 years ago

You're writing code in Go to generate a theme in Elisp? And you say you're not needlessly rewriting working code?

Again, it belongs in its own package, I say. But you've clearly made up your mind to do your experimentation here--in master, no less! :(

thomasf commented 4 years ago

It is not a part of the package, the resulting color alist is. I wrote it in Go becasue color.el doesn't have remotely near the feature that among others some Go packages have. I might port needed features to move color generation back into elisp later. I wrote it in Go specifically to avoid having to first write a color library first.

Maybe you are also interested to know that I also have written javascript to generate colors for this theme earlier for the same reasons. https://github.com/thomasf/solarized-work

Neither of these tools are anything that comes packaged with the theme but I did have problem just to day trying to find the right repository and revision used tog greate the -l and -d accent color versions. I didn't find them but I approximated them here now in the go code so they will at least exist in the same repository at the same point in time as they were created which might be relevant later on.

              pal.Accents.
            ChangeSaturation(0.2).
            Blend(pal.Base03, .2).
            ChangeLightness(-.1).
            NamedColors().
            WithSuffix("-d"),

        pal.Accents.
            ChangeSaturation(0.2).
            Blend(pal.Base2, .2).
            ChangeLightness(.05).
            NamedColors().
            WithSuffix("-l"),

The problem I am trying to solve it to understand if these colors even have a real place or not anymore .

conao3 commented 4 years ago

@alphapapa, If it is more convenient to write in go, you should not be accused of that fact. If he is working on a master and he has not broken anything, you should not accuse of that fact. You shouldn't dampen his motivation unnecessarily with minor mistakes (Merging my PR). It won't benefit anyone.

thomasf commented 4 years ago

All of this palette generation work got started after I spend a good portion of a couple of days reviewing this unified diff/hl change. ( https://github.com/bbatsov/solarized-emacs/pull/341#issuecomment-549181748 ) @alphapapa

I also started building a small set of tools to be able to do easy reviews/testing on pull requsts any my own changes. It's just simple stuff like generating the same merge conflict every time, having some exampe files and a emacs config that is likley to show off theming problems. ( https://github.com/bbatsov/solarized-emacs/blob/master/minimal-init/init.el )

While working the diff review it was yet again obvious to me that we needed new blends of colors for the diff's. I have had a go at it a few times before by just experimenting directly with the code inside the theme but the results never really that good. But this time we had solarized-theme-alist that just had been merged so I figured I make use of those and just go for the color manipulation library that I guessed would give me the best results fast and found that Go package and went with that.

Almost all code in there was written to generate those diff/diff-refine, it is only written in a way that lets me express what I want to do fast and reasonably clear to read.

As it happens just adding the color values from the other palettes and running the generator with exactly the same setting that are tuned for solarized just helped me reach results fast

For the first time ever in this themes history we have a magit diff/refine color scheme that actually makes me want to read the diffs in magit I could not be happier with the outcome, it is easy on the eyes, it looks very similar to the surrounding fg on bg text and it is resonably near solarizeds own 50% Lightness (of LAB) difference between foreground and background (it's about 40% but it's hard given that solarized by design already operates in a compressed range and it's very easy to make things stick out when they shouldnt.

alphapapa commented 4 years ago

For the first time ever in this themes history we have a magit diff/refine color scheme that actually makes me want to read the diffs in magit I could not be happier with the outcome, it is easy on the eyes, it looks very similar to the surrounding fg on bg text and it is resonably near solarizeds own 50% Lightness (of LAB) difference between foreground and background (it's about 40% but it's hard given that solarized by design already operates in a compressed range and it's very easy to make things stick out when they shouldnt.

You ought to show this amazing new feature in some screenshots in the readme.

thomasf commented 4 years ago

These screenshots are near how it looks now.. Using ediff merge there because it shows 4 different diff/refine variants at once https://github.com/bbatsov/solarized-emacs/pull/348#issuecomment-549935824 (keep scrolling down) . Having gruvbox as an easy switch to compare with while I was working was actually very helpful. I mean I have known for a long time that the problem always has been that the blending needed to happen separatley for dark/light themes and I know by earlier experience that doing it directly in elisp seldom has been a convenient way to get that kind of work done.

There have been a lot of looks-related that I have spend a lot of time on..
Believe it or not the default mode-line/header-line looks took a lot longer to arrive at then you might think. I tried lots of stuff over many sessions before I found a way to make it look flat with the help of underline instead of Emacs actual box drawing to get one emphasized buffer separator between buffers instead of several or none.

I tend to avoid screenshots beacuse they are easily outdated.. But I am planning on adding automatic screenshotting to my theme dev env/init.el where known files also exists, it could certianly be used to regenerate a bunch of screenshots so that they become easy to maintain.

bbatsov commented 4 years ago

It saddens to me see such negatively charged conversations taking place here. I haven't paid much attention to solarized recently, but I understand the sentiment of too much complexity for something as simple as a colour theme. Generally it seems to me that some theme infrastructure should be decoupled from the actual theme and names like solarized-zenburn for instance are super confusing. I guess we can package together the basics in something like base-theme and potentially distribute it separately. @purcell Would you be open to having a reusable theme foundation package on MELPA?

thomasf commented 4 years ago

Yes, the theme name is confusing. In hindsight its obvious that I should have thought more about how to present that situation. It was intentionally done both by namespace and maybe to force people to think about what it means before choosing to use any of those alternative palette variations.

However, solarized-zenburn is not straight up zenburn based themes and it never will be. Regardless of what others have been saying they are in fact the same theme (in a more abstract sense than a file named -theme.el ). The design, color selection will always be made usng the solarized palette primarily. Other zenburn themes which have zenburn as an origin in both palette and theme will always be able to do a "more" zenburn interpretation.

Since OG zenburn doesnt have an violet it is created by blending zb blue and magenta to get it to fit into the theme so it's tilted towards solarized in more ways than one.

Just to be clear: I never argued against removing those from the solarized MELPA package either in this discussion. I only tried to explain why I made that decision regardless if it was right or wrong.

I was just considering locking this issue but I unpinned it instead for now.. It started off so badly, nowhere near my stated intentions. Too much uncalled hostility, from what I can tell no attempt was ever even made to ask if the original insuperable event of adding an extra argument to a function was a mistake/oversight or not.

This made me loose a lot of enthusiasm in general for the time being but who knows, maybe it comes back.

bbatsov commented 4 years ago

Don't take this personally - OSS has always had this toxic vibe attached to it. Trust me - I'm an expert. :-) Accidents do happen, mistakes get made, we shouldn't make a drama out of them, but learn how we can do things better.

However, solarized-zenburn is not straight up zenburn based themes and it never will be. Regardless of what others have been saying they are in fact the same theme (in a more abstract sense than a file named -theme.el ). The design, color selection will always be made usng the solarized palette primarily. Other zenburn themes which have zenburn as an origin in both palette and theme will always be able to do a "more" zenburn interpretation.

Fair enough. This has to be documented somewhere, though, as it's definitely not obvious. I was under the impression that much of this work was done with the intention the just reuse the list of faces known to a theme to create easily other themes with different palettes. Clearly I got this wrong. For me when I was still working actively on Solarized and Zenburn it was always annoying I had to make pretty much the same edits to both themes.

Just to be clear: I never argued against removing those from the solarized MELPA package either in this discussion. I only tried to explain why I made that decision regardless if it was right or wrong.

👍

I was just considering locking this issue but I unpinned it instead for now.. It started off so badly, nowhere near my stated intentions. Too much uncalled hostility, from what I can tell no attempt was ever even made to ask if the original insuperable event of adding an extra argument to a function was a mistake/oversight or not.

I totally get it. I also considered just locking this, as it seemed it was going nowhere, but decided to to join the convo and try to help. It certainly doesn't seem like we're dealing with some insurmountable issue. :-)

bbatsov commented 4 years ago

All the themes are in the same directory as a file named solarized-theme.el, this file is picked up by load-theme but fails to load. That file should probably not exist.

Agreed.

Or, if you want a different bundle theme like doom-theme, you will be welcomed to create a new package with a package name like solarized-misc-themes and place the file there.

This sounds like a reasonable solution, although we can probably come up with a better package name.

thomasf commented 4 years ago

The multi palette feature also let me split the solarized into two separate alists with just colors to create better combinations for diff/refine bg/fg+bg/fg (4 colors) for dark and light. If nothing else I am at least very pleased that with the help of the last two pull requests and additional color blending work been able to produce these diff colors that are actually easy to read and nice to look at.

image

image

purcell commented 4 years ago

@purcell Would you be open to having a reusable theme foundation package on MELPA?

@bbatsov Yes, definitely.

t-soliduslink commented 4 years ago

I am bitten by the change to solarized-with-color-variables, too. It is not quite obvious what the right incarnation is to have it do the same that it did a week ago.

I.e. what do I have to replace

(solarized-with-color-variables 'light ...)

with in HEAD?

Thanks!

conao3 commented 4 years ago

In short:

(solarized-with-color-variables 'light ...)

;; to

(eval `(solarized-with-color-variables 'light ,solarized-light-color-palette-alist ...))
t-soliduslink commented 4 years ago

Thank you!

alphapapa commented 4 years ago

In short:

(solarized-with-color-variables 'light ...)

;; to

(eval `(solarized-with-color-variables 'light ,solarized-light-color-palette-alist ...))

@bbatsov Please revert this change to the macro.

This change has made this very useful macro difficult and obtuse to users.

There must be a better way. If necessary to simplify implementations elsewhere in the code, another function or macro could be written for that purpose, without breaking users' configurations in the wild.

thomasf commented 4 years ago

In my view the damage potential is already done, if we revert changes now we will rebreak the configurations for people that already have adapted to the changes.

Please stop posting comments in this thread if you have nothing new or constructive to say or I will begin to moderate your comments. We know your position already. You are disturbing the discussion this issue was created for.

alphapapa commented 4 years ago

In my view the damage potential is already done, if we revert changes now we will rebreak the configurations for people that already have adapted to the changes.

There are likely fewer people who have already adapted their configurations than who have yet to experience the breakage or have not had time to investigate. This is plain because the breakage is such that it requires visiting this issue discussion to find out how to fix it.

Please stop posting comments in this thread if you have nothing new or constructive to say or I will begin to moderate your comments. We know your position already.

I did not address you, I addressed @bbatsov. As far as I know, he still owns the project and I am free to address him. If he shares your opinion, or if he has turned it over to you, I will leave you to it. Otherwise, threatening censorship of polite, reasoned criticism does not bode well for the future of this project.

thomasf commented 4 years ago

retitled this issue version FUTURE because what is going to be 2.0 is what is in master now and might be ready soonish (within a few months or so). Also adopted an semantic version and adced note about it in the readme.

alphapapa commented 4 years ago

FYI:

https://github.com/hlissner/emacs-doom-themes/issues/314#issuecomment-570546890 https://github.com/hlissner/emacs-doom-themer