feat: allow multiple colorschemes #54

sheimer · 2024-06-23T19:19:10Z

if multiple colorschemes are used - e.g. colorscheme per filetype like folke/styler.nvim - tint resets to wrong colorscheme on untint and tints only with the default colorscheme

with this commit i read the current hl_ns of a window and build a new tinted hl_ns if needed while remembering to which namespace to switch on untint

if multiple colorschemes are used - e.g. colorscheme per filetype like folke/styler.nvim - tint resets to wrong colorscheme on untint and tints only with the default colorscheme

levouh · 2024-06-24T14:50:38Z

Do you have a "minimal-ish" config that I can use to test this @sheimer? I do not have anything beyond one global namespace for color schemes. Based on your comments, it looks like I could just setup styler.nvim to use a different color scheme for a certain file type and go from there?

lua/tint.lua

levouh · 2024-06-24T14:53:18Z

lua/tint.lua

+    tint_ns_id = __.tint_ns
+  else
+    local ns_suffix
+    for ns_name, ns_id in pairs(vim.api.nvim_get_namespaces()) do


Is this necessary? The docs for nvim_create_namespace say:

Namespaces can be named or anonymous. If `name` matches an existing namespace, the associated id is returned. If `name` is an empty string a

which if the goal here is to try to find an existing namespace or not, we can likely just call this function to "create or get", right? I have not gone through the code thoroughly yet so my interpretation of what you are trying to do here might be wrong.

not necessary for the changes to work.

While debugging I found a lot of hl-namespaces, and all had pretty clear namespace names, so everyone can figure out where they are coming from. Thats why I decided to go with a specific name depending on found colorscheme :)

levouh · 2024-06-24T14:54:01Z

lua/tint.lua

  end
 end

+local function add_namespace(ns_id, suffix)
+  __["tint_ns_" .. suffix] = vim.api.nvim_create_namespace("_tint_dim_" .. suffix)


I do not see that we are ever deleting these. I haven't used this patch yet so I am not sure how often they will build up, but I can imagine a scenario where this might be an issue?

on tints init only the default tinted scheme will be built, and tint is not aware of any schemes that might be loaded by styler on opening a specific filetype.

so i test a window for having a different ns_id as the default 0, then test if there is already a tinted version (if not _["tint_ns" .. ns_suffix]) and only then add another tinted namespace.

It should add up only depending on the number of colorschemes a user adds e.g. by using styler :)
I dont do extra checks to prevent tinting of an already tinted colorscheme...maybe I should add that, since "tint.tint" can be executed by user on any window, already tinted or not...

ok, I forgot I put the windows "untint_ns_id" to a window var, which prevents double tinting.

This window var gets deleted on restore_default_highlight_namespaces and untint, to allow switching of scheme if windows buffer and filetype change.

I found a problem with neo-tree
It sets a namespace on the window, but anonymous with id only, and creates a new id on each focus. So you were right, many new tint-namespaces got created.

For now I changed to only create a new namespace, if a name can be found.
Maybe it would be better to only create additional namespaces if a function "add_tint_function" returns true? (similar to window_ignore_function).

sheimer · 2024-06-24T17:00:22Z

Do you have a "minimal-ish" config that I can use to test this @sheimer? I do not have anything beyond one global namespace for color schemes. Based on your comments, it looks like I could just setup styler.nvim to use a different color scheme for a certain file type and go from there?

    {
      "zenbones-theme/zenbones.nvim",
      dependencies = {
        "rktjmp/lush.nvim",
        "neanias/everforest-nvim",
        "sheimer/styler.nvim",
      },
      priority = 1000,
      lazy = false,
      config = function()
        require("everforest").setup({})

        vim.cmd("colorscheme forestbones")

        require("styler").setup({
          themes = {
            markdown = { colorscheme = "everforest" },
            help = { colorscheme = "everforest" },
          },
        })
      end,
    },

this would be my colorscheme init. loading of tint is pretty much defaults.

styler.nvim needs to be my fork, because otherwise styler overwrites the ns of tint on WinNew events :) I have a merge request there too.

With this config, you can open an md file and some other file in two windows, then leave the "md" window (tinted highlights of default scheme) and get back to "md" window (default scheme on "md" window)

I hope this helps, but if you prefer i can prepare a minimal complete config for nvim.

sheimer · 2024-06-24T17:19:44Z

thank you very much for your review!
I tried to answer as best as I could...I will get to work on your suggestions as soon as possible (might be some days until I find the time :) )

dont create new tint namespaces for anonymous namespaces

allows user to decide if tint should create an extra tinted namespace defaults to function() return false end

sheimer · 2024-06-26T15:05:16Z

in my last commit i am going with a function to be set by the user, which decides if an additional namespace will be created.
i am not sure if that really makes sense in the end...but at least it allows me to allow tinted versions of styler-created namespaces only.
if you dont like the idea of this function and dont have a good idea how to build this multi-tint variant, maybe i could provide a smaller patch, where only the correct "original" namespace of a window is used on untint.

just let me know,

kind regard,

Swen

sheimer · 2024-07-07T09:10:27Z

Hi there :)

I hope I am not troubling you...yesterday folke accepted changes on styler

folke/styler.nvim@7005fa7

so styler will not untint tinted windows anymore.
Maybe you can have anothre look now?

Kind regards,

Swen

levouh · 2024-07-07T13:24:31Z

@sheimer Apologies for the delay. I am not as active on this project as I should be, for sure.

Are you able to update the DOC.md as well with some notes about the new configuration option? If you then run make docs (see Makefile) it runs md2vim to convert it to docs/tint.txt (so no need to update tint.txt directly). I don't have this in the CI right now, so if installing md2vim is too much of a pain I am also happy to do it this is merged.

Can we also make sure to document the main parameters of the new function and what exactly each of them means (winid, untint_ns_id, ns_suffix)?

levouh · 2024-07-07T13:02:17Z

lua/tint.lua

+  local untint_ns_id = get_untint_ns_id(winid)
+
+  local tint_ns_id
+  if untint_ns_id == 0 then


nit: Can simplify things here a bit by doing:

if untint_ns_id == 0 or not __.user_config.should_create_extra_tint(winid, untint_ns_id, ns_suffix) then

to make the function logic a bit more readable here - either we are just using the global namespace or we are using the user-enabled stuff.

I think you are right and it is better to make it more readable. For your suggestion I would need the ns_suffix always, so i decided to go a different way...to accomplish readability.
The result uses the "should_create_extra_tint" method only if no __.tint_ns_<originalid> exists already. To simplify a little more, i renamed the defaults tint_ns to __.tint_ns_0 - please let me know if you prefer the division to tint_ns and extra_namespaces

levouh · 2024-07-07T13:04:58Z

lua/tint.lua

+local function add_namespace(hl_ns_id, suffix)
+  local ns_name = "tint_ns_" .. tostring(suffix)
+  local ns_id = vim.api.nvim_create_namespace("_tint_dim_" .. tostring(suffix))
+  __[ns_name] = ns_id


nit: We could do something like __.extra_namespaces just to make sure things are organized in our __ map.

see above, i like the generalisation like tint_ns_<id> - let me know if you prefer otherwise

levouh · 2024-07-07T13:25:28Z

I will install this version and styler.nvim shortly so I can go through and test this a bit.

levouh · 2024-07-07T13:33:22Z

lua/tint.lua

@@ -39,6 +41,9 @@ __.default_config = {
    focus = { "WinEnter" },
    unfocus = { "WinLeave" },
  },
+  should_create_extra_tint = function()


What is an example of how you would use this function, and its arguments?

winid makes sense - you may want to get the &ft of a window and determine if you want it to be tinted differently.

I do not understand why the user would need untint_ns_id and ns_suffix to be passed to them.

good point :) in my latest draft, i use only one param, and it is the "untint_ns_id", so the original ns_id of the current window. I think it might be more logical, since i now only call the user-method, if no tinted version of ns_id already exists. so the "decision" really depends on the ns_id, not the window.

Still, from users perspective the decision by winid (and in consequence e.g. by filetype) seems easier to understand and implement.

My current version:

should_create_extra_tint = function(ns_id) local hl_ns_name = nil for ns_name, hl_ns_id in pairs(vim.api.nvim_get_namespaces()) do if hl_ns_id == ns_id then hl_ns_name = ns_name break end end if hl_ns_name ~= nil and string.find(hl_ns_name, "styler_") then return true else return false end end,

...so yeah, I guess I will refactor and send winid instead. What do you think, should the function be called always, per window, or only if no extra_namespace exists?

arg for should_create_extra_tint is now winid, example see DOC.md or doc/tint.txt

levouh · 2024-07-07T13:37:08Z

lua/tint.lua

+  __[ns_name] = ns_id
+
+  -- first copy over global tint namespace
+  for hl_group_name, hl_def in pairs(get_highlights(__.tint_ns)) do


Why do we need to copy over the global namespace first?

Because I found out that a window with a different Namespace then the default one still uses the default namespace, and the extra namespace is adding/overwriting existing highlights.
https://neovim.io/doc/user/api.html#nvim_win_set_hl_ns()

levouh · 2024-07-07T13:46:46Z

@sheimer I went through and tested this and all looks good as far as I can tell. I have some pending questions above (the nit ones are up to you) and the pending note about updating the docs.

sheimer · 2024-07-08T20:49:57Z

@sheimer Apologies for the delay. I am not as active on this project as I should be, for sure.

Are you able to update the DOC.md as well with some notes about the new configuration option? If you then run make docs (see Makefile) it runs md2vim to convert it to docs/tint.txt (so no need to update tint.txt directly). I don't have this in the CI right now, so if installing md2vim is too much of a pain I am also happy to do it this is merged.

Can we also make sure to document the main parameters of the new function and what exactly each of them means (winid, untint_ns_id, ns_suffix)?

Yes I guess I can, if i have trouble i will ask :)
I will try, if the final version of the user method is done, right now it is sending the ns_id, and maybe just the win_id is better really

sheimer · 2024-07-10T19:01:48Z

All done now. My final concern now would be: I really dont like the function name "should_create_extra_tint"...any suggestions? maybe "create_additional_tint_ns"?

feat: allow multiple colorschemes

0824a38

if multiple colorschemes are used - e.g. colorscheme per filetype like folke/styler.nvim - tint resets to wrong colorscheme on untint and tints only with the default colorscheme

levouh reviewed Jun 24, 2024

View reviewed changes

sheimer and others added 4 commits June 24, 2024 21:34

renamed get_global_highlights

032db7a

renamed variable, added type annotations

cad48bd

only add additional namespace if name can be found

ef55f76

dont create new tint namespaces for anonymous namespaces

new user_config param should_create_extra_tint

124f97e

allows user to decide if tint should create an extra tinted namespace defaults to function() return false end

levouh reviewed Jul 7, 2024

View reviewed changes

simplified get_tint_ns_id and refactored some variable names

c83cfba

sheimer added 2 commits July 10, 2024 20:39

should_create_extra_tint argument now winid and added to DOC

4ef7ab5

should_create_extra_tint defaults to nil

29befb8

fixed default return value of get_tint_ns_id

1a7428f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow multiple colorschemes #54

feat: allow multiple colorschemes #54

sheimer commented Jun 23, 2024

levouh commented Jun 24, 2024

levouh Jun 24, 2024

sheimer Jun 24, 2024

levouh Jun 24, 2024

sheimer Jun 24, 2024

sheimer Jun 24, 2024

sheimer Jun 24, 2024 •

edited

Loading

sheimer commented Jun 24, 2024

sheimer commented Jun 24, 2024

sheimer commented Jun 26, 2024

sheimer commented Jul 7, 2024

levouh commented Jul 7, 2024

levouh Jul 7, 2024

sheimer Jul 8, 2024 •

edited

Loading

levouh Jul 7, 2024

sheimer Jul 8, 2024

levouh commented Jul 7, 2024

levouh Jul 7, 2024

sheimer Jul 8, 2024

sheimer Jul 10, 2024

levouh Jul 7, 2024

sheimer Jul 8, 2024

levouh commented Jul 7, 2024

sheimer commented Jul 8, 2024

sheimer commented Jul 10, 2024

feat: allow multiple colorschemes #54

Are you sure you want to change the base?

feat: allow multiple colorschemes #54

Conversation

sheimer commented Jun 23, 2024

levouh commented Jun 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sheimer Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

sheimer commented Jun 24, 2024

sheimer commented Jun 24, 2024

sheimer commented Jun 26, 2024

sheimer commented Jul 7, 2024

levouh commented Jul 7, 2024

Choose a reason for hiding this comment

sheimer Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

levouh commented Jul 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

levouh commented Jul 7, 2024

sheimer commented Jul 8, 2024

sheimer commented Jul 10, 2024

sheimer Jun 24, 2024 •

edited

Loading

sheimer Jul 8, 2024 •

edited

Loading