Tinted nvim rewrite

[tummetott]

This PR is a complete rewrite of tinted-nvim.

While working with the existing codebase I ran into several architectural limitations that made it difficult to fix bugs and add new features cleanly. After reviewing the plugin in depth I concluded that a redesign was the most sustainable path forward.

What changed

This rewrite addresses long standing issues and introduces a more idiomatic Neovim plugin architecture.

Notable improvements include

• Very readable and extensible architecture
• Tinty integration no longer spawns tinty current as a subprocess
  The plugin now reads the Tinty artifact directly
• Fixes syntax reset inside set_highlights() re-sources colorscheme and causes double apply #32 vim.g.terminal_color_0 wrong base_color? #31 and ColorScheme autocmd not triggered when using custom colors. #25
• Emits the standard ColorScheme autocommand instead of a custom event
• Optional highlight compilation for faster startup (loads in 1.2 second with my macbook)
• Guards legacy highlight groups behind vim.fn.has("nvim-<version>")
• Automatically loads highlight definitions from lazy.nvim plugin specs
• Supports overriding existing schemes and creating custom schemes
• Follows standard Neovim plugin conventions
  require("tinted-nvim").setup(opts)
• Dont dupliacte palette specs in colors/<scheme-name>.vim. Just call the load(<scheme-name>)
• Uses LUA types
• Adding custom scheme artifacts generated by Tinty to lua/tinted-nvim/palettes/ together with a minimal loader in colors/ enables first class support for custom schemes. This allows custom themes to be loaded both via :colorscheme and require("tinted-nvim").load(""), and also integrates cleanly with automatic scheme switching when using Tinty’s watcher. This approach likely resolves Tinty custom generated schemes not recognized #18 and tinty apply with custom schemes breaks README.md and colorscheme.txt with t #33 as well although this has not been fully tested yet.

And much much more. Please see the README and the code for full details. I am also happy to answer questions or explain design decisions.

Breaking change

This is intentionally a breaking change.

The previous configuration format diverged significantly from common Neovim plugin conventions. Rather than incrementally patching around those differences I opted to realign the plugin with established patterns and provide a cleaner long term API.

To make this transition easier I suggest the following release strategy

• Tag the current implementation as v0.1.x
• Release this rewrite as v1.0.0

Users who want to stay on the old behavior can pin their plugin manager to

version = "0.*"

Closing thoughts

I spent a considerable amount of time designing implementing and documenting this rewrite. I believe it provides a much stronger foundation for future features and maintenance.

Thank you for taking the time to review this PR.

[tummetott]

Did you have a chance to look into this?

[bezhermoso]

Hi @tummetott Thank you for this!

This is a massive PR. Please consider breaking it down into smaller ones so it's easier to review & discuss specifics. Overall I'm for refactoring the plugin post fork, but its tough to see how we can merge this as-is in a reasonable time: I think we'll have a much better time & make better progress if we can merge in improvements/refactors piecemeal.

[tummetott]

Hi @bezhermoso, thanks for taking the time to review and for the feedback.

I understand the concern about the PR looking very large. One clarification that might help: the “1497 changed files” in the diff is mostly due to the colorscheme palette files. Those are templated and automatically generated from the tinted-schemes, and the actual change there is limited to a small template adjustment mainly line breaks for readability. There’s no expectation to review those files individually.

If you exclude the generated palettes, the functional changes are concentrated in a relatively small number of Lua modules. Each module has a single, well-scoped responsibility and can be reviewed independently. The reason I kept this as a single PR is that the pieces rely on a shared set of assumptions and a unified architecture; splitting it up would likely make individual PRs harder to understand without the full context.

That said, I definitely want to make this as easy to review as possible. I’m very happy to:

• Walk through the architecture and key design decisions in writing
• Add more rationale or comments to any unclear sections
• Do a guided code walkthrough on a call if that’s helpful

Another option, if it helps reduce risk, would be to simply try running it for a bit. The rewrite is intended to be drop-in usable, and using it day to day might be a quicker way to validate whether the new structure and API feel sane before going deep into line-by-line review.

I’m also fine treating this as a high-level v1 direction discussion first (API shape, conventions, overall design) before going into details.

Let me know what would work best for you.

[tummetott]

One additional bit of context I should have mentioned: the main goal of this PR was to fix the plugin’s internal architecture rather than introduce isolated features or refactors. That meant redefining some core invariants around module boundaries, responsibilities, and how schemes and highlights are loaded.

Once those invariants change, most of the code necessarily moves together, which is why this ended up as an atomic change instead of a series of incremental PRs. The intent was to make the architecture “right” first, so future changes and fixes can be done incrementally again.

Happy to explain or walk through any of those architectural decisions in more detail if that helps.

[JamyGolden]

Thanks for going through the effort of creating this. Since it's a large change it will take longer than other PRs to review and merge since it will probably be a more PR collaborative with @bezhermoso and I.

I've been busy recently but I'll go through this properly when I can. I've quickly looked through the file structure and noticed you removed ./lua/lualine/theme/tinted.lua. This file is needed, it is dynamically loaded by lualine which allows for lualine theme changes based on the theme set with tinted-nvim.

[JamyGolden]

@tummetott @bezhermoso I went through this and pushed a bunch of changes to this PR. Comment on any changes for discussions. I separated the different changes by commit to make it easier to go through.

[tummetott]

@JamyGolden I dug into the lualine theme logic. Right now, lualine checks the active colorscheme name and, if it starts with base16-, it set its theme to the generic base16 theme (see the snippet here).

That base16 theme is effectively a catchall for multiple Base16 implementations, for example:
https://github.com/RRethy/base16-nvim/
https://github.com/tinted-theming/tinted-vim
https://github.com/chriskempson/base16-vim
and tinted-nvim (this repo)

As a fallback, it also reads the BASE16_THEME environment variable. The full logic lives here

In practice, this implementation feels brittle and incomplete. Instead of relying on name heuristics and env vars, lualine should ideally detect actual lua modules (plugin presence) and pick the right integration explicitly.

The major issue today is that base24-* schemes are not recognized at all, since the detection only targets base16-*. More broadly, a single generic theme for “all Base16 plugins” may not be the best model.

My suggestion is:

1. Merge this PR first and update to the new tinted-nvim architecture
2. Then I can follow up with a lualine PR to improve detection and add a proper tinted-nvim theme integration
3. I create a PR for this plugin to add a lualine theme once detection works

[tummetott]

The actual theme definition itself is very simple:

local colors = require('tinted-nvim').get_palette_aliases()

local theme = {
    normal = {
        a = { bg = colors.grey, fg = colors.background, gui = 'bold' },
        b = { bg = colors.dark_grey, fg = colors.foreground },
        c = { bg = colors.darkest_grey, fg = colors.bright_grey }
    },
    insert = {
        a = { bg = colors.blue, fg = colors.background, gui = 'bold' },
        b = { bg = colors.dark_grey, fg = colors.foreground },
        c = { bg = colors.darkest_grey, fg = colors.foreground }
    },
    visual = {
        a = { bg = colors.yellow, fg = colors.background, gui = 'bold' },
        b = { bg = colors.dark_grey, fg = colors.foreground },
        c = { bg = colors.darkest_grey, fg = colors.background }
    },
    replace = {
        a = { bg = colors.red, fg = colors.background, gui = 'bold' },
        b = { bg = colors.dark_grey, fg = colors.foreground },
        c = { bg = colors.darkest_grey, fg = colors.foreground }
    },
    command = {
        a = { bg = colors.green, fg = colors.background, gui = 'bold' },
        b = { bg = colors.dark_grey, fg = colors.foreground },
        c = { bg = colors.darkest_grey, fg = colors.background }
    },
    inactive = {
        a = { bg = colors.darkest_grey, fg = colors.grey, gui = 'bold' },
        b = { bg = colors.darkest_grey, fg = colors.grey },
        c = { bg = colors.darkest_grey, fg = colors.grey }
    }
}

return theme

The main limitation is that this does not support cterm colors for non truecolor terminals. This is not really a theme issue but a lualine design limitation. Instead of passing the dict directly to nvim_set_hl(), lualine adds its own abstraction layer, which makes mixing gui and cterm attributes impossible.

Ideally, something like this should work, but it does not:

normal = {
    a = {
        bg = colors.grey,
        fg = colors.background,
        ctermbg = 8,
        ctermfg = 0,
        gui = 'bold',
    },
}

A clean workaround would be to switch to highlight group based definitions:

normal = {
    a = 'LualineNormalA',
    b = 'LualineNormalB',
    c = 'LualineNormalC',
}

and then define those groups properly in highlights/lualine.lua, including both gui and cterm values.

That said, I still think the right next step is to merge this PR first. We can then build proper lualine support on top with a follow up PR later.

[tummetott]

@JamyGolden OK, never mind. I have already added lualine support. It is not applied automatically yet because the detection mechanism is missing in lualine upstream. Once this PR is merged, I will follow up with the corresponding PR to lualine.

I also added a commit that guards truecolor support behind a config option. With this change, the plugin now works properly in non truecolor terminals as well.

[JamyGolden]

@tummetott it doesn't need to be in upstream lualine. You can read through a closed PR I added there a while ago: nvim-lualine/lualine.nvim#1409

[tummetott]

@JamyGolden I know and it currently isn’t. Please look at the last commit in this PR. I added the lualine scheme here, and that scheme is not in lualine upstream.

The real blocker is lualine’s upstream detection logic. Their auto theme detection only recognizes base16-* schemes, so base24-* schemes are not detected correctly. To have lualine pick this up automatically, we need a PR in lualine that fixes or restructures the detection.

As the lualine maintainer noted in the commit you linked:

“Also base16 theme here is getting overloaded a lot. Probably need a restructure if we are going to support so many different variants of base16 plugins.”

That’s exactly what I’m planning to do: improve the detection logic upstream so base24 schemes and the newer plugin variants are handled properly.

[JamyGolden]

Ah I see I hadn't looked at lualine/lua/themes/auto.lua logic before and yeah I see it's constrained to base16 prefixes. The lualine theme using highlight groups is nice.

[tummetott]

Let me know if you need anything else from to merge the PR

[JamyGolden]

Would be good to get an approval from @bezhermoso since it's such a large change.

[JamyGolden]

If bez hasn't responded tuesday ping me and we can merge it (I'll probably remember but in case I don't)

[bezhermoso]

Hi, apologies for the delay in responding, will make sure I take a deep look this weekend.

[bezhermoso]

Fantastic! This is full of great ideas that I’m excited about. A couple of general feedback:

(1) I think it’d also nice to provide Lua syntax to refer to colors in .build(…) by either color name and/or purpose e.g. scheme.RED/scheme.VARIABLES in place of ”red”
(2) More of guidelines for PRs in the future, @JamyGolden, artifacts should be generated as the last step before merge (or as separate PR, or only to be done by the bot post-merge). I know this is probably unavoidable for this refactor PR, but it surfaces how poor the review experience is on GitHub web. 😅

[bezhermoso]

Also, we want to think strategize how to make this change as smooth as possible for existing users:

(1) vim.notify when we detect if user is running an incompatible setup function. For example, I got this:

Failed to run `config` for tinted-nvim

/Users/bez/.config/nvim/lua/plugins/appearance.lua:94: module 'tinted-colorscheme' not found:
	no field package.preload['tinted-colorscheme']
	cache_loader: module 'tinted-colorscheme' not found
	cache_loader_lib: module 'tinted-colorscheme' not found
	no file './tinted-colorscheme.lua'
	no file '/opt/homebrew/share/luajit-2.1/tinted-colorscheme.lua'
	no file '/usr/local/share/lua/5.1/tinted-colorscheme.lua'
	no file '/usr/local/share/lua/5.1/tinted-colorscheme/init.lua'
	no file '/opt/homebrew/share/lua/5.1/tinted-colorscheme.lua'
	no file '/opt/homebrew/share/lua/5.1/tinted-colorscheme/init.lua'
	no file '/Users/bez/.luarocks/share/lua/5.1/tinted-colorscheme/init.lua'
	no file '/Users/bez/.luarocks/share/lua/5.1/tinted-colorscheme.lua'
	no file './tinted-colorscheme.so'
	no file '/usr/local/lib/lua/5.1/tinted-colorscheme.so'
	no file '/opt/homebrew/lib/lua/5.1/tinted-colorscheme.so'
	no file '/usr/local/lib/lua/5.1/loadall.so'

# stacktrace:
  - .config/nvim/lua/plugins/appearance.lua:94 _in_ **config**
  - .config/nvim/lua/core/lazy.lua:20
  - .config/nvim/lua/init.lua:4
  - lua:1

We should trigger the notification whenever someone lands an importable that used to exist.

(2) We should park this on a 1.x branch. Merge this in this form there, and polish the upgrade flow there before merging to main e.g. update the README, vim.notify guiding users to :help tinted-nvim-migrate-config, actually write that help section, etc.

[tummetott]

@bezhermoso

(1) I think it’d also nice to provide Lua syntax to refer to colors in .build(…) by either color name and/or purpose e.g. scheme.RED/scheme.VARIABLES in place of ”red”

Yep, I see your point. Currently the build() function already receives the palette argument, so you can define a highlight group like fg = palette.base0A. I mainly use aliases for better readability.

I agree that magic strings like "red" are not really Lua like. Instead, I suggest we add another argument to the build() function so the alias palette is passed in as well. Then, inside the highlight files, you can use either palette or aliases.

function M.build(palette, aliases, cfg)
  return {
    -- Using aliases
    DiagnosticError = { fg = aliases.red },
    DiagnosticWarn  = { fg = aliases.orange },
    -- Using palette
    DiagnosticInfo  = { fg = palette.base0A },
    DiagnosticHint  = { fg = palette.base08 },
  }
end

We could also merge aliases and palette before the function is called and only pass a single argument. However, I think that would mix semantics, so I’d prefer not to do that.

On aliases.RED uppercase: I would avoid that. Our alias vocabulary is lowercase, so introducing uppercase variants mixes semantics and adds duplication and noise. Keeping aliases lowercase is clearer and more consistent imho.

If you like this approach, let me know and i write the fix

[tummetott]

@bezhermoso

(1) I think it’d also nice to provide Lua syntax to refer to colors in .build(…) by either color name and/or purpose e.g. scheme.RED/scheme.VARIABLES in place of ”red”

completed in 503cdfa

We should trigger the notification whenever someone lands an importable that used to exist.

completed in 59522dc

(2) We should park this on a 1.x branch. Merge this in this form there, and polish the upgrade flow there before merging to main e.g. update the README, vim.notify guiding users to :help tinted-nvim-migrate-config, actually write that help section, etc.

I suggest we treat this as a major-version reset rather than trying to map old config to new config.

• tinted-nvim is effectively a new plugin/API surface, so we should encourage users to reconfigure from docs instead of adding brittle option-mapping logic. For most options, there exists no 1 to 1 map
• I already added the compatibility shim for require("tinted-colorscheme") in this PR
• The shim warning clearly say:
  • the plugin was renamed to tinted-nvim
  • the API/config changed
  • users should read :h tinted-nvim

What i would suggest in addition:

1. Keep old plugin line available as 0.5.x.
2. Release new plugin as 1.0.0.
3. Document the break explicitly in README (“Breaking changes in 1.0”, no automatic migration, pin 0.5 as fallback).
4. Keep shim warning in 1.x to guide users, then remove in a later release.

what do you think?

[bezhermoso]

100% agree that we don't need to keep configs interop. But we need to at least honor tinted-colorscheme as an import w/ a clear ramp to migrate, even if it does nothing else.

[tummetott]

100% agree that we don't need to keep configs interop. But we need to at least honor tinted-colorscheme as an import w/ a clear ramp to migrate, even if it does nothing else.

Thats already done. Did you check the commits I did yesterday ?