navarasu / onedark.nvim

One dark and light colorscheme for neovim >= 0.5.0 written in lua based on Atom's One Dark and Light theme. Additionally, it comes with 5 color variant styles
MIT License
1.61k stars 164 forks source link

`setup()` refactoring #37

Closed xeluxee closed 2 years ago

xeluxee commented 3 years ago

Hi I think we should improve setup function to let users configure the entire plugin with it, using a config dictionary as argument However, since the plugin should be configurable trough vimscript too, I think we could do keep a dictionary called g:onedark_setup, to be configured before calling the setup function. All options would then be moved under this dictionary, except for onedark_style Also custom highlights should be added as a setup option, solving issues #35, maybe #33, #23, maybe #17 and #12

@navarasu if you agree with these changes or you have some suggestion let me know, so I'll make a PR

navarasu commented 3 years ago

Let's not break existing configs. We can support both legacy and new options. But we can remove the old config details from README.

" Option 1  - Old one  (Priority 2)
let g:onedark_style = 'darker'
let g:onedark_transparent_background = v:true

" Option 2 (Priority 1)
let g:onedark_option = { 'theme':  'darker',  transparent: v:true }
colorscheme onedark
-- Option 3  - Old one (Priority 2)
vim.g.onedark_style = 'deep'
vim.g.onedark_italic_comment = false

-- Option 4 (Priority 1)
 require('onedark').setup {
       theme = 'darker',
       transparent = true,
       toggle_theme_key = '<leader>ct',
       terminal_colors = true, -- True enable the terminal
       hide_ending_tilde = true
       styles = {
         comment = 'italic'
       },
       diagnostics = {
         undercurl = true,
         darker_color = true
       }
 }
xeluxee commented 3 years ago

So you'd like to keep g:onedark_* options? Maybe we could keep them for a month or so, printing warning messages to tell users that these options will be deprecated in favor of the config dictionary (g:onedark_setup or g:onedark_options) or the lua setup() function...

navarasu commented 3 years ago

Yes. We can print a deprecated message and support g:onedark_* options for the next 4 months. So we support both options with the above said priority

navarasu commented 3 years ago

@xeluxee As part of this ticket, we can just implement new setup option and support existing config. No need to solve the issues mentioned in description. We can do that after you complete this migration.

Meanwhile I will work on transparency based on float value and adding light theme.

xeluxee commented 3 years ago

@navarasu I agree Since in this period I'm busy I won't finish my PR these days, so we'll have to wait a bit before it gets ready to be merged

xeluxee commented 2 years ago

Before proceeding, can you tell me if this dictionary looks good or if you have any setting to add?

local default_config = {
    toggle_style_keymap = '<leader>cs',
    italics = {
        comments = true,
        keywords = false,
        functions = false,
        strings = false,
        variables = false
    },
    bolds = {
        comments = false,
        keywords = true,
        functions = false,
        strings = false,
        variables = false
    },
    contrast_windows = {
        -- specify which windows get the contrasted (darker) background
        "NvimTree",
        "terminal",
        "packer",
        "qf",
    },
    diagnostics = {
        darker = true,        -- darker colors for diagnostics
        undercurl = true,     -- use undercurl for diagnostics
        background = true,    -- use background color for virtual text
    },
    disable = {
        background = false,     -- don't set background (NeoVim then uses your teminal background). If you want transparent background set it to true and enable transparency in your terminal's settings
        term_colors = false,    -- prevent the theme from setting terminal colors
        eob_lines = false       -- hide the end-of-buffer tildes
    },

    custom_highlights = {}
}

This configuration was heavily inspired by material.nvim

navarasu commented 2 years ago

@xeluxee Looks fine. But we can avoid too many child parents. This nightfox.nvim config looks cleaner.

Let's have a config like this,

local default_config = { 
    -- Main options --
     theme = 'dark', 
     transparent = false,  -- don't set background 
     term_colors = true, -- True enable the terminal
     ending_tilde = false,  -- show the end-of-buffer tildes
     toggle_theme_key = '<leader>ct',

     -- Changing Styles --
     styles = {
       comment = 'italic',
       keywords = 'NONE',
       functions = 'NONE',
       strings = 'NONE',
       variables = 'NONE'
     },

    -- Custom Highlights --
     colors = {}, -- Override default colors
     highlights = {}, -- Override highlight groups

    -- Plugins Related --
     diagnostics = {
        darker = true, -- darker colors for diagnostic
        undercurl = true,   -- use undercurl for diagnostics
        background = true,    -- use background color for virtual text
     },
 }
xeluxee commented 2 years ago
toggle_theme_key

Wouldn't toggle_style_key be better? Since that keymap doesn't change theme but it changes onedark style

navarasu commented 2 years ago

Yeah. You are right. Lets keep it as toggle_style_key. Also, we can change the theme to style. Then styles params have to be changed. So we can call it format. Then the config will be as,

local default_config = { 
    -- Main options --
     style = 'dark', 
     toggle_style_key = '<leader>ts',
     toggle_style_list = ['dark', 'darker', 'cool, 'deep', 'warm', 'warmer'],
     transparent = false,  -- don't set background 
     term_colors = true, -- True enable the terminal
     ending_tilde = false,  -- show the end-of-buffer tildes

     -- Changing Formats --
     format = {
       comment = 'italic',
       keywords = 'NONE',
       functions = 'NONE',
       strings = 'NONE',
       variables = 'NONE'
     },

    -- Custom Highlights --
     colors = {}, -- Override default colors
     highlights = {}, -- Override highlight groups

    -- Plugins Related --
     diagnostics = {
        darker = true, -- darker colors for diagnostic
        undercurl = true,   -- use undercurl for diagnostics
        background = true,    -- use background color for virtual text
     },
 }
xeluxee commented 2 years ago

https://neovim.io/doc/user/syntax.html#attr-list

What about changing format to attributes or hlattributes?

navarasu commented 2 years ago

Not meaningful. Nvim means it as highlight attributes for cterm. What about text_format or text_style? I remember a very old vim plugin with that name like txtfmt

xeluxee commented 2 years ago

Not meaningful. Nvim means it as highlight attributes for cterm.

If you scroll down a bit (or :h highlight-gui) you'll see that these attributes are the same for cterm and gui

What about text_format or text_style?

Good, but what about code_style?

xeluxee commented 2 years ago

And what about contrast_windows?

navarasu commented 2 years ago

And what about contrast_windows?

That we can implement pluginwize later and I have other bigger ideas. Planning to add multiple styles for lualine, nvimtree, etc.

First, we can implement existing configs and bring in this new setup concept. Then we can add necessary features one by one like custom highlights, text formatting, etc.

navarasu commented 2 years ago

Good, but what about code_style?

Yes. Meaningful

navarasu commented 2 years ago

Today I added better screenshots for the existing dark themes in the NEW_README .

Next, I will update the configuration section for the new config based on the above discussion.

xeluxee commented 2 years ago

https://github.com/navarasu/onedark.nvim/blob/c1f739280bbd8b7d6183491ca95f73195baecf83/README.md?plain=1#L7 Instead why not using [Installation](#installation)? This is a markdown notation, so the page isn't reloaded

xeluxee commented 2 years ago

Today I added better screenshots for the existing dark themes in the NEW_README .

Next, I will update the configuration section for the new config based on the above discussion.

Looks great!

navarasu commented 2 years ago

@xeluxee I am a little free for the next two days. Shall I work on this ticket?

xeluxee commented 2 years ago

I'll be available for this in two weeks or so @navarasu sorry for the delay but in this period I'm really busy

Shall I work on this ticket?

If you wish you can start refactoring or making light theme in the meantime