Merge lightline + adjust lightline & airline #72

dsifford · 2018-04-12T02:10:41Z

WIP. Don't merge yet

This PR merges the lightline theme over from the lightline repo to here as well as adjusts the colors of both lightline and airline so that they use the same exact palette as the main theme.

Lightline merge is a co-authored commit with @adamalbrecht so he gets credit for his work.

TODO:

Finish lightline
Finish adjusting airline

benknoble

So far, all systems go. Will review airline code when pushed.

A quick test (without lightline) shows no breakages; would recommend someone who uses lightline test just in case.

benknoble · 2018-04-12T10:53:58Z

autoload/lightline/colorscheme/Dracula.vim

+  let s:p.normal.error = [ [ s:red, s:black ] ]
+  let s:p.normal.warning = [ [ s:yellow, s:black ] ]
+
+  let g:lightline#colorscheme#Dracula#palette = lightline#colorscheme#flatten(s:p)


I won't pretend to know anything about lightline, although I spent some time browsing the repo/code and it looks nice. Assuming this works, LGTM, but I haven't experimented with it or its colors.

Yeah, I had no experience with it either.

I downloaded it and made sure it worked while after I wrote this in so I can confirm that it looks at least as good as it did before. And I can also confirm that placement of the lightline plugin with respect to this theme doesn't matter either when loading using (at least) vim-plug.

benknoble · 2018-04-12T10:54:49Z

colors/dracula.vim

@@ -53,6 +53,20 @@ let s:yellow    = ['#F1FA8C', 228]

 let s:none      = ['NONE', 'NONE']

+let g:dracula_palette = {


This will be super useful for other plugin integrations (looking at you, airline). Good call.

I think so to, thanks. I ended up adding in the extended colors as well. Figured some other UI themes might want to use those as well. 👍

dsifford · 2018-04-13T03:45:05Z

Thanks for the review @benknoble 🙏 🙏

Just finished up the airline update. Check it out and let me know what you think. I added a couple little helper functions in the airline file that makes it easier to use our palette with their requirements. It also makes it a bit more readable.

Side note: Assuming you didn't change your mind, I'm gonna add you as a maintainer here to this theme in the readme file (tomorrow at some point).

You have push/pull access here, right?

benknoble

Glad to hear lightline doesn't have any issues; I think there's a problem with the Airline inactive section I'd like to see fixed, but if you'd rather I open a new issue for that (as opposed to fixing it in this PR) that's ok too.

I do think we should document the functions, just for readability.

benknoble · 2018-04-13T09:37:30Z

autoload/airline/themes/dracula.vim

-let g:airline#themes#dracula#palette.visual.airline_error = s:ERROR
-let g:airline#themes#dracula#palette.replace.airline_error = s:ERROR
-
-" File modified and not saved


Are we keeping a "file modified and not saved" section in the color scheme ? I couldn't find one below.

I removed it because the color that was defined was so closely similar to the regular section that I couldn't really even notice a difference.

Also, on my system, once a file is changed, it switches over to the _modified variant permanently, even after it is saved, so it didn't seem very useful.

Thoughts?

Actually, I did notice it because it when from a dark black/purple color to a grey (with the little [+]). The grey and the little [+] go away after I save.

With the current patch, the middle of the status line is just always the grey modified color.

Hm.. maybe my system is different or neovim handles it differently because I definitely didn't notice that.

The color that was used for the "gray" was out of spec. Thoughts on using the bgdark color instead if I add it back in?

Fine by me.

Final thought: Would it be useful/helpful to also enable italics for changed filenames if the user has italics enabled?

shrugs I could go either way on that one, since I don't use them. Is that typical in other IDEs? Your call.

That's what they do in VS Code. But I don't particularly find it useful or necessary. I generally know if a buffer has been modified or not if I try to quit out and it doesn't let me.

Agreed. Vim let's us know in it's own way. That's one reason I like the color change/indicator. I'm so used to it I don't have to look at it; the corner of my eye sees the change and it's instant feedback.

Cool. So we'll keep as-is for the time being then.. Merging!

benknoble · 2018-04-13T09:39:38Z

autoload/airline/themes/dracula.vim

+"}}}
+" Helpers: {{{
+
+func! s:clr(fg, bg, ...)


I would enjoy seeing some documentation (read: comments) on the usage of the script-local functions, here and elsewhere, since Vimscript isn't necessarily the easiest thing to read and we're dealing with a very specific data structure for airline. For future maintenance, it could be nice.

benknoble · 2018-04-13T09:40:03Z

autoload/airline/themes/dracula.vim

+
+"}}}
+
+let g:airline#themes#dracula#palette = {


Very clean; love it.

benknoble · 2018-04-13T09:55:59Z

autoload/airline/themes/dracula.vim

+\         'airline_error': s:clr('bg', 'red'),
+\       },
+\     ),
+\   'inactive': s:color_map(


In inactive mode (i.e., with multiple windows open) the airline on regular buffers (not help, quickfix, or netrw with my testing) is unreadable in the left gutter. It appears the text becomes the same color as the grey background, becoming impossible to read.

Actually, come to think of it, this was present before this commit... is this expected behavior ?

Screenshot?

Actually, ignore this. I found my problem. What happens is airline collapses the mode and branch sections, leaving the file name. I had been experimenting on a new file without a name. Editing a named file is the same as it was (readable but subtle and not in the way).

benknoble · 2018-04-13T10:01:45Z

@dsifford As for the maintainer-ship, I'm still interested. I have pull access, but I don't think push since I'm not in the 'vim team' ?

I'll request to join...

dsifford · 2018-04-13T17:33:19Z

@benknoble Your suggestions have all been addressed. Let me know when you get a chance if we're good to merge

Thanks again 👍

dsifford · 2018-04-13T17:42:16Z

Added you to the readme @benknoble -- Feel free to adjust once @zenorocha gives you push access.

I added you as "David Ben Knoble" but, judging by your username, I assume you go by your middle name generally?

Co-authored-by: adamalbrecht <[email protected]>

benknoble

Everything LGTM. Correct about the naming. Either is fine, it's been a life of Ben, David, David-Ben, Ben-David, &c.

benknoble · 2018-04-13T17:19:51Z

autoload/airline/themes/dracula.vim

@@ -10,6 +10,9 @@
 "}}}
 " Helpers: {{{

+" Takes a foreground color name, background color name, and optionally one or


Excellent! Now I know what's going on!

@benknoble

* Updates team list * Fixes typo * Fixes link * Merge lightline + adjust lightline & airline (dracula#72) * add built-in lightline support Co-authored-by: adamalbrecht <[email protected]> * finish lightline merge * finish airline update * add inline doc comments to airline script functions * add back modified groups to airline * adjust maintainers + add @benknoble in README.md * remove lambda for better backwards compat. closes dracula#73

dsifford mentioned this pull request Apr 12, 2018

Request to take Dracula responsibility itchyny/lightline.vim#299

Closed

dsifford requested a review from benknoble April 12, 2018 02:12

benknoble approved these changes Apr 12, 2018

View reviewed changes

benknoble requested changes Apr 13, 2018

View reviewed changes

dsifford and others added 6 commits April 13, 2018 14:29

add built-in lightline support

3adc532

Co-authored-by: adamalbrecht <[email protected]>

finish lightline merge

88e62d8

finish airline update

ea0416b

add inline doc comments to airline script functions

80181ae

add back modified groups to airline

04d3281

adjust maintainers + add @benknoble in README.md

f2d84cd

dsifford force-pushed the dsifford-lightline-airline branch from 892b9ba to f2d84cd Compare April 13, 2018 18:31

benknoble approved these changes Apr 13, 2018

View reviewed changes

dsifford merged commit dc8073c into master Apr 14, 2018

dsifford deleted the dsifford-lightline-airline branch April 14, 2018 00:50

benknoble mentioned this pull request Mar 16, 2019

ALE warning/error background colors for Airline shift in different modes. #129

Closed

		@@ -53,6 +53,20 @@ let s:yellow = ['#F1FA8C', 228]

		let s:none = ['NONE', 'NONE']

		let g:dracula_palette = {

Merge lightline + adjust lightline & airline #72

Merge lightline + adjust lightline & airline #72

Uh oh!

Conversation

dsifford commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benknoble left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsifford commented Apr 13, 2018

Uh oh!

benknoble left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benknoble commented Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsifford commented Apr 13, 2018

Uh oh!

dsifford commented Apr 13, 2018

Uh oh!

benknoble left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dsifford commented Apr 12, 2018 •

edited

Loading

benknoble left a comment •

edited

Loading

benknoble commented Apr 13, 2018 •

edited

Loading