nordic.nvim icon indicating copy to clipboard operation
nordic.nvim copied to clipboard

refactor!: use vim.api.nvim_set_hl()

Open gegoune opened this issue 3 years ago • 9 comments

Completely broken.


Edited by @andersevenrud

  • [ ] Update examples
  • [ ] Add a git message with migration help
refactor!: use vim.api.nvim_set_hl()

Introduces changes in color definitions which is a BREAKING CHANGE
if you use the `custom_colors` callback function option.

[INSERT INSTRUCTIONS HERE WITH A SIMPLE EXAMPLE]

Closes #69

gegoune avatar Feb 01 '22 21:02 gegoune

Not sure why it's never going into table conditional in https://github.com/andersevenrud/nordic.nvim/pull/70/files#diff-87af52ac4b5511b195aa96dd5bdea0128de6210195d900717f6e55f58c004c66R111.

I am trying to make output from nordic functions to return objects that are ready to be injected into vim.api.nvim_set_hl().

gegoune avatar Feb 01 '22 21:02 gegoune

Ignore that comment. I clearly misread something in the docs. Stand by...

andersevenrud avatar Feb 01 '22 22:02 andersevenrud

Well, not that far off :sweat_smile: I was unaware of the naming and that the gui attribute is now split into properties.

                local definition = {
                    fg = group[2] or 'NONE',
                    bg = group[3] or 'NONE',
                    sp = group[5] or 'NONE',
                }

                if group[4] then
                    definition[group[4]] = true
                end

                vim.api.nvim_set_hl(0, group[1], definition)

One thing I don't know about here is underline though, because that's an option, and I don't know if underline supports a non-bool value :man_shrugging: I can't find the definition of this in the docs for some reason...

andersevenrud avatar Feb 01 '22 22:02 andersevenrud

Thanks! Will look into it tomorrow. But basically you are against changing group configurations the way I did in fidget.lua or standard.lua? Should make it easier for that very PR, but could get rid of those table[n] maps.

Thanks for looking into it!

gegoune avatar Feb 01 '22 22:02 gegoune

But basically you are against changing group configurations

Nope.

I just didn't understand the impact of using this function.

But now that I get it this should be fairly easy to do. The fastest way is to change the tables from this:

{ name | name[], fg, bg, style, sp }

to:

{ name | name[], { fg = '', bg = '', ... } }

Then finally:

    local function load_group(list)
        for _, group in ipairs(list) do
            if type(group) == 'function' then
                load_group(group(unpack(arguments)))
            elseif type(group[1]) == 'table' then
                load_group(vim.tbl_map(function(highlight)
                    return { highlight, group[2] }
                end, group[1]))
            else
                vim.api.nvim_set_hl(0, group[1], group[2])
            end
        end
    end

andersevenrud avatar Feb 01 '22 22:02 andersevenrud

FYI: I updated the description with some tasks

andersevenrud avatar Feb 01 '22 22:02 andersevenrud

There's actually an even simpler way for this, but I'm not sure how I feel about it because it involves using some black Lua magic to have a mix between a list and a map:

-- This works :o
local t = { "foo", fg = "bar" }
print(t[0])
print(t.fg)

Which means that technically one can do this:

{ name | name[], fg = '', bg = '', ... }
        for _, group in ipairs(list) do
            if type(group) == 'function' then
                load_group(group(unpack(arguments)))
            elseif type(group[1]) == 'table' then
                load_group(vim.tbl_map(function(highlight)
                    return { highlight, group }
                end, group[1]))
            else
                local definition = {}
                for k, v in pairs(group) do
                  if type(k) == 'string' then
                    definition[k] = v
                  end
                end
                vim.highlight.create(group[1], definition)
            end
        end

andersevenrud avatar Feb 01 '22 22:02 andersevenrud

I will come back to it, but most likely not in next couple of weeks unfortunately.

gegoune avatar Feb 03 '22 22:02 gegoune

Alright. Take your time :)

andersevenrud avatar Feb 03 '22 23:02 andersevenrud