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

Color swap #45

Merged
merged 26 commits into from Jan 17, 2020
Merged

Color swap #45

merged 26 commits into from Jan 17, 2020

Conversation

roosta
Copy link
Member

@roosta roosta commented Jan 11, 2020

OK, so this pull request pertains to all the pull requests from srcery-emacs and srcery-terminal as well.

Recently there was a discussion here, pertaining to certain elements in a terminal would be drawn using bright black, most notably comments, but this affect many different systems. The only solution was to redefine some colors, bright-black and white, brightening both colors. That works but leaves a problem, we need a substitute for bright-black, and as it happens xgray1 was a good fit.

That means that some color swapping needed to be done. I introduce a new x color (xgray6) and shift all the xgray colors upward xgray5 -> xgray6, xgray4 -> xgray5 and so forth. When this is done, I swap all instances of bright-black with xgray1, and all instances of white with bright-black.

This update doesn't really change the appearance too much in my opinion, but I really need feedback on this, if this even works.

One more thing is that we now have a white color that is left unused. I figure we'll find some use for it at some point.

here is an image illustrating the problem
image

and here is after this change
image

I'll use this branch going forward to make sure any unforeseen doesn't crop up, and leaving this pull request hanging for a little bit.

@MindTooth
Copy link
Member

Did some preliminary testing with kitty & Vim.

kitty

Old

Screenshot 2020-01-12 at 12 05 30

New

Screenshot 2020-01-12 at 12 05 55

Vim

Old

Screenshot 2020-01-12 at 12 21 44

New

Screenshot 2020-01-12 at 12 24 16

Seems to me that the highlight color is darker, leaning more towards gray. Highlight and vertical wrap bar is the same color I presume?

fish

Did not du much for fish, but I do believe that the color is static and not dynamic. I'll maybe look into creating a scheme in the future.
Screenshot 2020-01-12 at 12 06 03

tmux

My scheme took a dive for the worse. 😝 No biggy.

Screenshot 2020-01-12 at 12 27 09

@roosta
Copy link
Member Author

roosta commented Jan 12, 2020

@McSinyx there is a few differences in color, white is now brighter, bright-black is now what white used to be and there is one new color (xgray6).

@MindTooth The swap doesn't seem to play nice with your transparent background color, its near impossible to see the highlight and vertical wrap.
I had to do some finagling to get my own tmux theme to work as well, the process I used in airline and lightline theme is like in this pull request, shifting all the grays up a notch starting at 5->6 4->5 and so forth, and then finally replacing bright-black with gray1 and white with bright-black.

Its possible that a simple config change in https://github.com/zsh-users/zsh-syntax-highlighting could also do the trick. I'll have to look into it more. I'm still on the fence about whether this pull request is a good idea or not.

@roosta
Copy link
Member Author

roosta commented Jan 12, 2020

@MindTooth you are right that highlight and vertical wrap are the same color, they are now both gray1

@roosta
Copy link
Member Author

roosta commented Jan 12, 2020

OK, seems you can set comment color for zsh-syntaxt-hightlighting fairly easily with this config option:

ZSH_HIGHLIGHT_STYLES[comment]='fg=white'

But it still leaves issues with other systems:
image

@MindTooth
Copy link
Member

I don’t mind the new highlight color. I’ve yet to test with new colors in fish.

Anything you need from me? How is your own experience with the change so far?

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

Glad you don't mind, was a bit uncertain since the new highlight color doesn't show up well in your transparent terminal.

If you could keep trying this change for a bit longer I'd appreciate it. I'm doing the same to make sure everything works properly. As it stands my experience have been good, no big issues yet. I was thinking that I might change the white color to be a bit brighter, so as to differentiate it better from the bright black.

Not ready to merge just yet, but so far it looks like its working.

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

Shame that it didn't work well with fish shell, let me know if you figure out a solution. Is there some config option to set the colorscheme manually?

@MindTooth
Copy link
Member

Well, I believe I did spoke to soon. Not been putting any attention on the color as I've been using formatters anyway. 🤣

Now that I actually went looking, I do in fact struggle to see it. Turning off transparency does solve the issue. Not a permanent solution though. 😉

An idea could be to add a flag to the config so that one might enable a brighter color for users that do use transparent terminals.

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

A config option might be an idea, but I worry that if we choose another gray color it will conflict with something else. I'll look into it.

What bothers me a bit is that I really like the way it is in master, bright black should be a dim black color in my opinion, and white should be a text color (comment, header), but this is not consistent with a lot of terminal apps

I don't really know the best way of solving this, and if it doesn't work in fish anyways that makes me more hesitant.

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

Turns out the issue in zsh with completion color can be easily fixed

ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE="fg=7"

Seems there are fixes for most of these color issues

@MindTooth
Copy link
Member

I can use this command:

set fish_color_autosuggestion B6A88D
# or
set fish_color_autosuggestion white

That seems to set it permanently.

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

That's nice, that leave only npm and kitty autocomplete that I know of, I'll have to look into that later.

@roosta roosta closed this Jan 16, 2020
@roosta roosta reopened this Jan 16, 2020
@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

Oops, was not suppose to close this, had a few github windows opened and interacted with the wrong one.

@roosta
Copy link
Member Author

roosta commented Jan 16, 2020

I did some more work on this, checkout the new highlight color and see if it works for you @MindTooth. I rather prefered it to the darker version

I looked into the original xterm palette and see that bright black is indeed supposed to be a gray variant:
image

On consideration I think I wanna go ahead with this merge, so as to conform as closely to the original palette variations as we can.

@MindTooth
Copy link
Member

Screenshot 2020-01-17 at 10 51 44

Screenshot 2020-01-17 at 10 52 48

I'm happy with the latest change.

@roosta
Copy link
Member Author

roosta commented Jan 17, 2020

Alright, merging then. Everything seems to be working. Keep an eye out for vim issues, I'm mainly on emacs so might take me a while to notice.

@roosta roosta merged commit d76e510 into master Jan 17, 2020
@roosta roosta deleted the color-swap branch January 17, 2020 17:31
barskern added a commit to barskern/dotfiles that referenced this pull request Jan 19, 2020
MindTooth pushed a commit to srcery-colors/srcery-shell that referenced this pull request Jan 19, 2020
* Add skin for broot

* Modify broot skin to use new srcery colors

srcery-colors/srcery-vim#45
@MindTooth
Copy link
Member

After some review, as I'm updating the website, I can't seem to find that you did in fact shift the grays? Were this left out, or is the current implementation correct.

@roosta
Copy link
Member Author

roosta commented Feb 20, 2020

I did as I described in the first comment:

I introduce a new x color (xgray6) and shift all the xgray colors upward xgray5 -> xgray6, xgray4 -> xgray5 and so forth

Are you not seeing evidence of this?

@MindTooth
Copy link
Member

MindTooth commented Feb 20, 2020

Ahh, you ment in the schema for Vim. 😅 I read it as the for the colors them self. My bad. 😉

@roosta
Copy link
Member Author

roosta commented Feb 20, 2020

Ah I see, no I didn't actually change any of the grays, just shifted up one step :)

pull bot referenced this pull request in tkntjsdw/srcery-vim Sep 9, 2020
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.

None yet

3 participants