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

fix(compiler): compile all themes inst of 1 mult times #290

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

tmillr
Copy link
Member

@tmillr tmillr commented Jul 31, 2023

Fix bug where the current theme gets compiled multiple times instead of compiling all themes.

@tmillr tmillr marked this pull request as draft July 31, 2023 20:15
@tmillr
Copy link
Member Author

tmillr commented Aug 1, 2023

I had to make this a draft because there seems to be negative side-effects that were either produced, or simply made visible, by this pr. For example (airline highlights changed):

Screenshot 2023-08-01 at 1 13 49 AM



I need to investigate this further but it might have something to do with the color lib? Not sure. Lmk if you have any ideas. It seems that the order in which the themes are compiled might matter (due to some unforeseen side-effects of compilation).

Edit: Yep, just confirmed that it's sensitive to compilation order. So this is easily fixed for the time being by simply compiling the currently-set theme last. There's obviously some relevant global/persistent state somewhere that is being set or altered during compilation causing side-effects. This isn't optimal (and this global state should probably be refactored out in the future if possible), but perhaps it can be worked-around for now? I think that either airline, or the color module, is having its state altered during compilation (for example, the Color lib/module has persistent/global values such as Color.BG).


Explanation of the bug

if cached ~= hash then
M.compile()
write_file(cached_path, hash)
end

  1. Within setup() in lua/github-theme/init.lua, when a compilation is triggered, M.compile() is called.

function M.compile()
require('github-theme.lib.log').clear()
local compiler = require('github-theme.lib.compiler')
local themes = require('github-theme.palette').themes
for _, style in ipairs(themes) do
compiler.compile({ style = style })
end
end

  1. M.compile() loops over all of the themes, calling compiler.compile({ style = style }) on each one.

function M.compile(opts)
opts = opts or {}
local theme = config.theme
local spec = require('github-theme.spec').load(theme)
local groups = require('github-theme.group').from(spec)
local background = spec.palette.meta.light and 'light' or 'dark'
local lines = {
fmt(
[[
return string.dump(function()
local h = vim.api.nvim_set_hl
if vim.g.colors_name then vim.cmd("hi clear") end
vim.o.termguicolors = true
vim.g.colors_name = "%s"
vim.o.background = "%s"
]],
theme,
background
),
}
if config.options.terminal_colors then
local terminal = require('github-theme.group.terminal').get(spec)
for k, v in pairs(terminal) do
table.insert(lines, fmt([[vim.g.%s = "%s"]], k, v))
end
end
for name, values in pairs(groups) do
if should_link(values.link) then
table.insert(lines, fmt([[h(0, "%s", { link = "%s" })]], name, values.link))
else
local op = parse_styles(values.style)
op.bg = values.bg
op.fg = values.fg
op.sp = values.sp
op.blend = values.blend
table.insert(lines, fmt([[h(0, "%s", %s)]], name, inspect(op)))
end
end
table.insert(lines, 'end)')
opts.name = theme
local output_path, output_file = config.get_compiled_info(opts)
util.ensure_dir(output_path)
local file
if vim.g.github_theme_debug then
file = io.open(output_file .. '.lua', 'wb')
file:write(table.concat(lines, '\n'))
file:close()
end
file = io.open(output_file, 'wb')
local f = loadstring(table.concat(lines, '\n'), '=')
if not f then
local tmpfile = util.join_paths(util.get_tmp_dir(), 'github_theme_error.lua')
require('github-theme.lib.log').error(
fmt(
[[There is an error in your github-theme config.
You can open '%s' for debugging.
If you think this is a bug, kindly open an issue and attach '%s' file.
Bellow is the error message:
]],
tmpfile,
tmpfile
)
)
local efile = io.open(tmpfile, 'wb')
efile:write(table.concat(lines, '\n'))
efile:close()
dofile(tmpfile)
end
file:write(f())
file:close()
end

  1. Nowhere within the body of compiler.compile(opts) is opts.style ever referenced; instead, config.theme is used as the theme to compile (which as I understand is simply the currently-set theme).

The implications of all of this is that whenever a compilation is triggered and M.compile() in lua/github-theme/init.lua gets called, the very same theme gets compiled and written to the filesystem (at the same path) multiple/many times in a row.

Solution

I'd suggest either fixing this to compile all of the different themes/styles (which appears to be the original intention), or perhaps changing it to compile just 1 theme (the currently-set theme) with others only being compiled impromptu on an as-needed basis. This pr currently attempts to do the former.

Fix bug where the current theme gets compiled multiple times instead of
compiling all themes.
@tmillr
Copy link
Member Author

tmillr commented Aug 16, 2023

Regarding the sensitivity to compilation order issue, this line appears to be the offending line:

local tbg = config.transparent and 'NONE' or s.bg0

in particular, the value of s.bg0 varies.

So it's probably something in spec.lua or palette.lua (and may still have something to do with the global/persistent state of the color lib).

@ful1e5 ful1e5 marked this pull request as ready for review December 2, 2023 10:11
@ful1e5 ful1e5 merged commit 3f29c8d into projekt0n:main Jan 31, 2024
17 checks passed
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.

None yet

2 participants