rktjmp / lush.nvim

Create Neovim themes with real-time feedback, export anywhere.
MIT License
1.49k stars 47 forks source link

Theme configuration #17

Closed rktjmp closed 3 years ago

rktjmp commented 3 years ago

https://github.com/rktjmp/lush.nvim/discussions/8#discussioncomment-435988

Wanting to define some optional or customizable highlighting in a just a few lines. You don't have to have a settings section and a highlighting section as in elisons gruvbox.nvim or even in my theme (I will send it to you shortly). (This is nice because, if you want to have a lots of settings you can have them in a section of a couple tens of LOC and not separated by another 400 lines):

config['italic_comments'] = vim.g.scheme_italic_comments == 1 and "italic," or ""

theme = lush(function()
    return {
       Normal { ... },
       ... -- lots of LOC
       Comment { ..., gui = config.italic_comments },
       ...
    }end)

instead, you can have it defined together... (maybe also in the same closure, so you do not even need to read the variable if you don't need it eventually in the scheme based on some condition (plugin not used or different language))

rktjmp commented 3 years ago

I think when extends is in mainline, that would solve this? Since everything is a module, you can do this in different files or not.

config = get_config()

-- default settings
base = lush(function()
  return {
    Normal { ... },
    Comment { fg: ... }
  }
end)

-- define the Comment group, using base.Comment but adjust for config
-- no extends, we just want to define a few groups
comments = lush(function()
  return {
    -- where comment_settings has been processed into the correct string (including NONE)
    Comment { base.Comment, gui = config.comment_settings } 
  }
end)

-- prepare to generate "end spec", order matters!
combined = {base, comments}

-- maybe adjust specs as needed
if config.load_rust then table.insert(combined, require 'theme.rust') end

-- generate what vim will actually see (api may not need unpack in release)
return lush.extends(unpack(combined)).with(function()
  return {
    -- may have no content or may define always set but not base groups
    LineNr { ... }
  }
end)
rktjmp commented 3 years ago

@kunzaatko

kunzaatko commented 3 years ago

OK, so the structure with this would be to:

  1. make the base theme
  2. Make a table with the extensions (through some conditions) like so: {base, extensions...}
  3. define the final scheme with
    .with(function()
    return{
        -- some definitions
    })

I do not understand the usefulness of with. You could rely on the base definitions in the extensions but not on the ones in with, but the definitions both in base and with are always applied... Why would I not define everything in base, when there is no difference in it? If it is so, then why not drop the with and only rely on extends, which would return the full spec? Sorry if I do not understand... @rktjmp

rktjmp commented 3 years ago

extend() tells lush "hey, remember these specs."

with() says, "here's a spec, build and return it. Also merge it onto these from extend".

Without calling with() you just have a list of specs waiting to be combined. It is functionally equivalent to lush() but lush.extends(base, another).lush(...) is weird to read; where as you can read this as "lush should extend these specs with this".

That's why it could be empty, it just has to be called to kick off the merge. I could make it so extends(a, b, c,)() worked like an empty call to with, but I would rather keep the magic to a minimum, there being one API style of extends->specs->with->spec :: parsed_spec.

The usefulness of with is for when I want to use your scheme, but some parts need changing:

nord = require('lush_theme.nord)

lush.extend(nord).with(function()
  return {
    -- i like nord but need brighter normal text
    Normal { fg = nord.Normal.fg.li(30) },
    Comment { fg = colorblind_safe_green } -- also I cant see normal color
    MyPlug { ... } -- and I want my custom local plugin to mirror some nord settings. 
  }
end)
kunzaatko commented 3 years ago

Right. I understand this after the explanation. Maybe the API could be more intuitive by renaming the methods so that this logic behind it is intuitively obvious from the names. I would suggest renaming:

rktjmp commented 3 years ago

Not sure I agree that "extend(these specs) with(this spec)" is less intuitive than "chain(these specs)" and "collect(this spec)", esp since chain would normally be related to functions and collect to lists you want to reduce (IMO). Maybe "collect(these specs) into(this spec)".

kunzaatko commented 3 years ago

Well, I thought of it more like a builder style API. I was thinking about it like you have a object (a spec), you can modify it by extending it and and then collecting it into a theme. Well that is a matter of preference I guess, I just think that the chain/extend, collect API is more intuitive. Maybe should I ask in #8 to get more opinions?

rktjmp commented 3 years ago

extends.with and merge now in mainline cc5ba4038e6b300a97dadf2b0f38c6d06505ccf7. https://github.com/rktjmp/lush.nvim#advanced-usage

Closing this as I believe it covers it, else re-open.