fix(compiler): consider entire config when hashing (#350)

- Hash entire config instead of just the table passed to `setup()`. This
  helps to ensure that a recompilation occurs when overrides are set
  outside of `setup()`.

- Loading/sourcing colorscheme now causes recompilation if config or
  overrides have changed, even if `setup()` has been called before.

- Clear `vim.g.colors_name` before setting `vim.o.background` so that
  colorscheme is not reloaded recursively when setting
  `vim.o.background` (as opposed to using a variable to check for nested
  colorscheme load, which is what we were doing before).

- fix(compiler): always write hash to filesystem when a compilation
  occurs, incl. when `require('github-theme').compile()` is called
  directly.

Related: #262, #340, #341

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 -   Improved highlight-group overrides (#349)
 -   Assigning `false` or an empty table to a highlight group clears it
 -   Assigning `false` to groups/specs/palettes clears previous settings from the config store
+-   Loading/sourcing colorscheme now causes recompilation if config or overrides changed, even if `setup()` has been called before
 
 ### Changes
 
@@ -21,10 +22,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Issues Fix
 
--   Fixed `punctuation.delimiter` treesitter group nearly invisible (#329 fixed by #331)
+-   Fixed `punctuation.delimiter` treesitter group nearly invisible (#329 fixed-by #331)
 -   Closed #305 (no longer valid, fixed)
 -   Closed #292 (no longer valid, fixed)
 -   fix(config): `options.darken.floats` is not used (#345)
+-   fix(compiler): consider entire config when hashing (#350) (related-to #262, #340, #341)
+-   fix(compiler): always write hash to filesystem when compilation occurs incl. when `require('github-theme').compile()` is called directly (#350)
 
 ## [v1.0.2] - 03 May 2023
 

lua/github-theme/_test/util.lua

@@ -1,4 +1,4 @@
-local M = {}
+local M = { unique_hlgroup = '____0' }
 local api = vim.api
 
 function M.await_VimEnter()

lua/github-theme/init.lua

@@ -1,4 +1,8 @@
 local config = require('github-theme.config')
+local override = require('github-theme.override')
+local keys = { 'palettes', 'specs', 'groups' }
+local did_setup = false
+local M = {}
 
 local function read_file(filepath)
   local file = io.open(filepath, 'r')
@@ -17,99 +21,91 @@ local function write_file(filepath, content)
   end
 end
 
-local M = {}
-
 function M.reset()
   require('github-theme.config').reset()
   require('github-theme.override').reset()
 end
 
 ---Compiles all themes/styles with their current settings.
+---@param force boolean true by default
 ---@return nil
-function M.compile()
-  require('github-theme.lib.log').clear()
-  local compiler = require('github-theme.lib.compiler')
-  local themes = require('github-theme.palette').themes
-  local current_theme = config.theme
-  for _, theme in ipairs(themes) do
-    -- Compile current theme last (see discussion in #290)
-    if theme ~= current_theme then
-      compiler.compile({ theme = theme })
-    end
-  end
-  compiler.compile({ theme = current_theme })
-end
+function M.compile(force)
+  local util = require('github-theme.util')
+  util.ensure_dir(config.options.compile_path)
 
--- Avoid g:colors_name reloading
-local lock = false
-local did_setup = false
+  local cached_path = util.join_paths(config.options.compile_path, 'cache')
+  local cached = read_file(cached_path)
+  local git_path =
+    vim.fn.fnamemodify(vim.fn.resolve(debug.getinfo(1).source:sub(2)), ':p:h:h:h')
+  local git = vim.fn.getftime(util.join_paths(git_path, '.git'))
 
-function M.load(opts)
-  if lock then
-    return
+  -- This is needed because neither `opts` nor `config` necessarily contain
+  -- everything we need to hash. For example, `opts` may not contain all
+  -- overrides and config currently in use (`setup()` might have been called
+  -- before, or maybe overrides were set directly and outside of `setup()`), and
+  -- `config` does not contain any of the overrides in use. Therefore, we need
+  -- to create a new table which contains everything in-use.
+  local dummy = { options = config.options }
+  for _, k in ipairs(keys) do
+    dummy[k] = override[k]
   end
 
-  if not did_setup then
-    M.setup()
+  local hash = require('github-theme.lib.hash')(dummy) .. (git == -1 and git_path or git)
+
+  if force ~= false or cached ~= hash then
+    require('github-theme.lib.log').clear()
+    local compiler = require('github-theme.lib.compiler')
+    local themes = require('github-theme.palette').themes
+    local current_theme = config.theme
+
+    for _, theme in ipairs(themes) do
+      -- Compile current theme last (see discussion in #290)
+      if theme ~= current_theme then
+        compiler.compile({ theme = theme })
+      end
+    end
+
+    compiler.compile({ theme = current_theme })
+    write_file(cached_path, hash)
   end
 
-  opts = opts or {}
+  getmetatable(override).__index.changed_since_last_compile = false
+end
 
+function M.load(opts)
+  opts = opts or {}
   local _, compiled_file = config.get_compiled_info(opts)
-  lock = true
-
   local f = loadfile(compiled_file)
-  if not f then
-    M.compile()
+
+  if not did_setup or override.changed_since_last_compile or not f then
+    M.setup()
     f = loadfile(compiled_file)
   end
 
   ---@diagnostic disable-next-line: need-check-nil
   f()
-
   require('github-theme.autocmds').set_autocmds()
-  lock = false
 end
 
-M.setup = function(opts)
-  did_setup = true
+---Applies any new config or overrides then (re)compiles if needed.
+---@param opts? table
+function M.setup(opts)
   opts = opts or {}
-
-  local override = require('github-theme.override')
+  did_setup = true
 
   -- New configs
   if opts.options then
     config.set_options(opts.options)
   end
 
-  if opts.palettes ~= nil then
-    override.palettes = opts.palettes
-  end
-
-  if opts.specs ~= nil then
-    override.specs = opts.specs
-  end
-
-  if opts.groups ~= nil then
-    override.groups = opts.groups
-  end
-
-  local util = require('github-theme.util')
-  util.ensure_dir(config.options.compile_path)
-
-  local cached_path = util.join_paths(config.options.compile_path, 'cache')
-  local cached = read_file(cached_path)
-
-  local git_path =
-    vim.fn.fnamemodify(vim.fn.resolve(debug.getinfo(1).source:sub(2)), ':p:h:h:h')
-  local git = vim.fn.getftime(util.join_paths(git_path, '.git'))
-  local hash = require('github-theme.lib.hash')(opts) .. (git == -1 and git_path or git)
-
-  if cached ~= hash then
-    M.compile()
-    write_file(cached_path, hash)
+  for _, k in ipairs(keys) do
+    local v = opts[k]
+    if v ~= nil then
+      override[k] = v
+    end
   end
 
+  M.compile(false)
   require('github-theme.util.deprecation').check_deprecation(opts)
 end
 

lua/github-theme/lib/compiler.lua

@@ -38,13 +38,16 @@ function M.compile(opts)
       [[
 return string.dump(function()
 local h = vim.api.nvim_set_hl
-if vim.g.colors_name then vim.cmd("hi clear") end
+if vim.g.colors_name then
+  vim.cmd("hi clear")
+  vim.g.colors_name = nil
+end
 vim.o.termguicolors = true
-vim.g.colors_name = "%s"
 vim.o.background = "%s"
-    ]],
-      opts.theme,
-      background
+vim.g.colors_name = "%s"
+]],
+      background,
+      opts.theme
     ),
   }
 

lua/github-theme/override.lua

@@ -1,9 +1,8 @@
-local collect = require('github-theme.lib.collect')
 local M = {}
 
 function M.reset()
   getmetatable(M).__index =
-    { palettes = {}, specs = {}, groups = {}, has_override = false }
+    { palettes = {}, specs = {}, groups = {}, changed_since_last_compile = false }
   return M
 end
 
@@ -43,6 +42,8 @@ setmetatable(M, {
     local store = getmetatable(self).__index
 
     if type(store[k]) == 'table' then
+      store.changed_since_last_compile = true
+
       if not v then
         store[k] = {}
       elseif k == 'groups' then

test/github-theme/config/compiler_spec.lua

@@ -0,0 +1,29 @@
+local assert = require('luassert')
+local t_util = require('github-theme._test.util')
+local uniqgrp = t_util.unique_hlgroup
+
+if vim.fn.has('nvim-0.9.0') == 0 or vim.fn.has('nvim-0.9.0') == false then
+  return
+end
+
+describe('compiler', function()
+  before_each(function()
+    require('github-theme.util.reload')(true)
+  end)
+
+  it('should consider previously-set overrides (1)', function()
+    local same_opts = {}
+    require('github-theme').setup(same_opts)
+    require('github-theme.override').groups = { all = { Normal = { link = uniqgrp } } }
+    require('github-theme').setup(same_opts)
+    vim.cmd.colorscheme({ args = { 'github_dark_dimmed' } })
+    assert.same({ link = uniqgrp }, t_util.get_hl('Normal', true))
+  end)
+
+  it('should consider previously-set overrides (2)', function()
+    require('github-theme').setup({})
+    require('github-theme.override').groups = { all = { Normal = { link = uniqgrp } } }
+    vim.cmd.colorscheme({ args = { 'github_dark_dimmed' } })
+    assert.same({ link = uniqgrp }, t_util.get_hl('Normal', true))
+  end)
+end)

test/github-theme/smoketest/startup_spec.lua

@@ -27,6 +27,7 @@ describe('(smoke test)', function()
         end)
 
         assert.is.equal('', vim.v.errmsg or '')
+        assert.is.equal(cs, vim.g.colors_name)
       end
     end)
   end)

test/minimal_init.vim

@@ -3,6 +3,6 @@ lua <<
     vim.loader.disable()
   end
 .
-set rtp+=.
-set rtp+=./test/plenary
+set rtp^=.
+set rtp^=./test/plenary
 runtime! plugin/plenary.vim