vim-airline / vim-airline-themes

A collection of themes for vim-airline
MIT License
2.04k stars 348 forks source link

Dracula inactive theme has only two colors #191

Closed andys8 closed 4 years ago

andys8 commented 4 years ago

Dracula theme looks like this to me, if the buffer is inactive. I'm not sure if it's intended to look like this

image

I played around with some colors and having at least some separation would be nice: image

https://github.com/vim-airline/vim-airline-themes/blob/ad6cc5261bff23a5b8d7232a971881f79909e767/autoload/airline/themes/dracula.vim#L71

chrisbra commented 4 years ago

it's inactive, so I would say this is intended.

chrisbra commented 4 years ago

BTW: feel free to add an option and create a PR for this (don't forget to document the option then).

get-me-power commented 4 years ago

What do you think this patch? @chrisbra

-let g:airline#themes#dracula#palette.inactive = airline#themes#generate_color_map(s:IA, s:IA, s:IA)
+
+if get(g:, 'airline#themes#dracula#inactive_simple', 1)
+  let g:airline#themes#dracula#palette.inactive = airline#themes#generate_color_map(s:IA, s:IA, s:IA)
+else
+  let g:airline#themes#dracula#palette.inactive = airline#themes#generate_color_map(s:IN1, s:IN2, s:IA)
+endif

screenshot

let g:airline#themes#dracula#inactive_simple = 1

スクリーンショット 2019-11-25 19 54 02

let g:airline#themes#dracula#inactive_simple = 0

スクリーンショット 2019-11-25 19 53 23
get-me-power commented 4 years ago

@andys8 I will try to create Pull request. Is it OK?

andys8 commented 4 years ago

@kazukazuinaina Of course, thanks a lot!

@chrisbra I tried some other themes and their inactive versions have (decent) colors. But Dracula currently has a single text and a single background color. I initially guessed vim-airline might be broken. I think the colors can be improved, and I'm fine with it, but in my opinion the flag/option might not be necessary.

get-me-power commented 4 years ago

Okey! I will create a Pull Request!

@andys8 and @chrisbra I think it's a good thing to have many choices, what do you think?

chrisbra commented 4 years ago

It's a matter of taste and it seems like the current behavior was intended by the author @joaovinicius

The question I have currently is:

We had too many complaints in the past, therefore, I'll accept only changes if they are documented and are configurable (unless it is clearly a bug or several people agree)

Either way, I don't mind too much and I am fine with changing it but since I am not good at graphics, I'd like to hear more opinions.

get-me-power commented 4 years ago

Thanks. @chrisbra I look forward to solving this problem in a better direction.

benknoble commented 4 years ago

I stopped using Airline some time ago... FWIW, my custom statusline makes use of dracula colors, but just uses bold/no-bold to highlight the active window (though I may revisit that).

The best place to check out "dracula colorscheme vim stuff" is the https://github.com/dracula/vim repo—also see https://dsifford.github.io/dracula-spec/

chrisbra commented 4 years ago

@benknoble thanks. I don't know if the suggested edit makes sense and fits the colorscheme.

@andys8 Can you please give feedback #192? thanks.

andys8 commented 4 years ago

The PR is exactly using the colors I had, when playing around and creating the first screenshot(s:IN1, s:IN2, s:IA). I don't have an idea how they were initially meant. As long as the PR is working it looks fine to me and I'd be happy to use it.

dsifford commented 4 years ago

So just chiming in with an observation. I haven't really dug into this issue thread all that hard, but I'm curious about this... Why does the airline repo also have a dracula configuration when we maintain one ourselves? Could this cause inconsistency issues? Ping @chrisbra

chrisbra commented 4 years ago

@dsifford no, because what airline provides is just a way to style the different parts of your statusline. Those should obviously match the global colorscheme, that's why we call those here themes and they only cover how the different parts in the statusline are colored.

chrisbra commented 4 years ago

oh, wait a second. So the dracula colorscheme already comes with a airline theme. Then we should indeed drop this one

chrisbra commented 4 years ago

I dropped this scheme. @andys8 please report your issue upstream then.

andys8 commented 4 years ago

Mhh, gonna have to try it. But: I'm using https://github.com/flazz/vim-colorschemes/blob/master/colors/dracula.vim instead of dracula/vim.

dsifford commented 4 years ago

Try the official one. You might be pleasantly surprised. 👍

andys8 commented 4 years ago

I'll try again. I tried it multiple times and the result in syntax hightlighting was worse (opinion, of course).

chrisbra commented 4 years ago

I agree, if possible, always try the upstream repository. Chances are, they can fix your problems early.

BTW: This was also the reason, I have never merged the various include a gruvbox airline theme PRs.

andys8 commented 4 years ago

@dsifford Left dracula/vim, right vim-colorschemes. image

andys8 commented 4 years ago

And more relevant for the issue: Yes, dracula/vim has airline styles. The inactive version looks like this (so same issue, different repo). image

dsifford commented 4 years ago

@andys8 Re: the first issue: That's because your terminal doesn't support full truecolor. So to achieve the proper colors on your degraded terminal, you should set let g:dracula_colorterm = 0 and possible disable termguicolors if you have that enabled.

Re: the second one. Open an issue and we can talk about it there. 

Related to your first issue: https://github.com/dracula/vim/issues/117

get-me-power commented 4 years ago

@dsifford

Thanks for giving suggestion for us!

get-me-power commented 4 years ago

@chrisbra

I thought this was a good decision.

It's a shame that my patch wasn't adopted, but it is my hope if vim-airline is going in a better direction.

I will continue to contribute to vim-airline, Thanks!!

chrisbra commented 4 years ago

@kazukazuinaina sorry, about that. But we really shouldn't diverge with different themes for the same colorscheme. Try proposing your patch to the dracula color scheme so it makes everybody happy 😄

dsifford commented 4 years ago

We're extremely open to PRs in the dracula repo so long as it conforms to the running spec (linked above by @benknoble ) and the dracula core scheme.

The sort of trickiness in maintaining a theme that is so widely used is that you have to force yourself to make decisions that don't stray too far from "sane" defaults. Lots of opinions out there and it's impossible to please everybody. If changes are proposed that stray a bit too far from our "sane default" philiosophy, we will generally tell the submitter to either add their proposed changes to their own .vimrc or to fork.

andys8 commented 4 years ago

I changed my theme to dracula/vim. I disabled italics (has to happen before the theme is set) and dracula_colorterm = 0 which results in a different background. The difference in syntax coloring was because of differences in elm syntax. I think the result is pretty okay for me :)

Regarding the vim-airline (the actual topic). It looks like this to me (notermguicolors).

left active, right inactive

image

andys8 commented 4 years ago

How about this for inactive airlines? image

\   'inactive': s:color_map(
\       ['bg', 'selection'],
\       ['fg', 'bgdark'],
\       ['bg', 'selection'],
\       {
\         'airline_warning': s:clr('bg', 'orange'),
\         'airline_error': s:clr('bg', 'red'),
\       },
\     ),
chrisbra commented 4 years ago

please request upstream.

andys8 commented 4 years ago

Question was targeted to @dsifford but you're right.