lunacookies / vim-colors-xcode

Xcode 11’s dark and light colourschemes for Vim
ISC License
608 stars 60 forks source link

Allow statement emphasis to be configured (default: enabled) #11

Closed dvdzkwsk closed 4 years ago

dvdzkwsk commented 4 years ago

I'd like to propose making statement emphasis configurable, since at least in TypeScript (but not JavaScript) it leads to a lot of emphasis on the screen at once. This may be a language-specific issue outside of the scope of your library, since many tokens that aren't statements are being colored as such (vim colorschemes are foreign to me, so this is the most straightforward way I could find to address the issue. If you know of a better fix I'd love to hear it).

Regardless, this option may still be welcomed by users who wish to avoid emphasis altogether. I briefly considered a broader option to disable all emphasis so that the options don't spiral out of control, but discarded that idea because it is valuable to see emphasized TODO's, which would mean you'd end up with an inverse option to re-enable that.

let g:xcodedark_emph_statements=1 (default)

Screen Shot 2020-02-29 at 9 15 26 AM

let g:xcodedark_emph_statements=0

Screen Shot 2020-02-29 at 9 15 47 AM

Thanks again for this awesome color scheme. This is my first time using vim-colortemplate, so please let me know if I missed anything, or if you'd prefer another approach; I'd be happy to correct it.

lunacookies commented 4 years ago

I’ve never used TypeScript, but that highlighting definitely doesn’t look ideal! Could you please tell me what the generally accepted TS plugin is so I can play around with the highlight groups?

I wouldn’t be opposed to adding an option to de-emphasise statements, but there is one problem with your implementation.

If you had recompiled the WWDC colourscheme it would have ended up with an unused g:xcodewwdc_emph_statements variable and a meaningless entry in its documentation because emph_statements doesn’t apply to it. I think the only way to solve this is ~to separate out the code for emphasisation into _emph_funcs, _emph_idents, _emph_types, and finally _emph_statements.~ I have separated these out into separate files in abea430.

dvdzkwsk commented 4 years ago

Oh dang, sorry about the WWDC color scheme. I had looked at it in the past and hadn't seen any bold, and so naively assumed it still didn't. My mistake.

As far as plugins go, I think I've stumbled upon the source of the issue: vim-polyglot includes https://github.com/HerringtonDarkholme/yats.vim, which sets let g:yats_host_keyword = 1 by default. Disabling that produces the desired highlighting.

With that, you're free to close this PR unless you still think this option is valuable, in which case I can make the necessary updates.

lunacookies commented 4 years ago

hadn't seen any bold, and so naively assumed it still didn't

No, your assumption was correct! The only problem is that the statement emphasisation code doesn’t apply to the WWDC colourscheme (specifically because it doesn’t have bold).

Disabling that produces the desired highlighting

Thank you so much for finding this. Initially I disabled it too, but I’ve realised that it is possible to make use of the extra highlighting that let g:yats_host_keyword = 1 provides without highlighting everything as keywords, so now I don’t think there is a need to set that to zero any more. I’ve just pushed some changes that hopefully make TypeScript highlighting more like the languages I’ve specifically added support for.

you're free to close this PR unless you still think this option is valuable

If you would still like to use the deemphasisation even with the new highlighting then feel free to update the PR, but otherwise I don’t think there is much of a need for it, especially since it’s so easy to add custom colourscheme modifications in your vimdrc:

augroup customColors
    autocmd!
    autocmd ColorScheme * hi Statement gui=NONE cterm=NONE
augroup END
dvdzkwsk commented 4 years ago

If you would still like to use the deemphasisation even with the new highlighting then feel free to update the PR, but otherwise I don’t think there is much of a need for it, especially since it’s so easy to add custom colourscheme modifications in your vimrrc:

The snippet you posted will do just fine, so I'll go ahead and close this. The updated TypeScript support is much better out of the box now anyways; thank you for that :).

lunacookies commented 4 years ago

Glad I could help :)