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

Add option to skip filetype-specific colors (again) #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DarkWiiPlayer
Copy link

Same as #119 but without indentation for easier diffing :)

Cool as they are, they lead to problems when switching to this
colorscheme and then back to a different one, thus an option to disable
them will make life easier for many users.
@pickfire
Copy link

pickfire commented Sep 26, 2018

@DarkWiiPlayer Creativity +1 😆 👍

@DarkWiiPlayer
Copy link
Author

*poke*

@NLKNguyen
Copy link
Owner

Hi,

Sorry for the late response, I didn't forget this. I originally planned to implement this feature in a different way but never got time to do so. I guess I can do it after Thanksgiving :).

Here are the reasons:

First of all, it's not advised to introduce the usage of an arbitrary global variable. I try to keep all options as key/value pairs within 1 single global variable for PaperColor: g:PaperColor_Theme_Options (see the README usage). And there is a mechanism in which the option only affects an intended scope, either only a variant (dark/light) or both variants of a theme whether it is the default PaperColor or some other theme; yes, it's possible to have multiple themes on PaperColor -- I didn't advertise this feature enough but here is an example in case you wonder https://github.com/NLKNguyen/papercolor-blue ).

What I have in mind is to introduce an option, say highlight_only_standard_groups or something. Every option defaults to 0, and users can set to 1 to enable what this PR intended to do. For example,

let g:PaperColor_Theme_Options = {
  \   'theme': {
  \     'default': {
  \       'highlight_only_standard_groups': 1
  \     }
  \   }
  \ }

In case you wonder how to do add the option

In the code, you see this. It's where you can introduce a new theme option.

fun! s:generate_theme_option_variables()
  " 0. All possible theme option names must be registered here
  let l:available_theme_options = [
        \ 'allow_bold', 
        \ 'allow_italic', 
        \ 'transparent_background',
        \ 'highlight_only_standard_groups',
        \ ]

A script variable will automatically be created as s:themeOpt_highlight_only_standard_groups, and it has the value of either 1 or 0 according to the config as expected.

Second, instead of having a big if-block like the PR, that function can be terminated right after the standard highlighting groups and be done.

if s:themeOpt_highlight_only_standard_groups == 1
  return
endif

Hope this helps. I'll implement this the next time I set up Vim again. Or if you implement as I suggested and test, then I can pull. Thanks!

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.

3 participants