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

fixed including an invalid parameter on highlights.lua (erased "create" parameter) #311

Closed
wants to merge 1 commit into from

Conversation

MowlandCodes
Copy link

@MowlandCodes MowlandCodes commented Dec 30, 2024

removed the parameter on nvim.api.nvim_get_hl('create' parameter) which is invalid

@rcarriga
Copy link
Owner

This is a valid parameter as far as I can see, what's the issue you're having exactly?

@MowlandCodes
Copy link
Author

In my Neovim it says that the "create" parameter is invalid and throwing an error, when i delete that parameter it runs normally, and I'm using a custom function to redirect all of my Neovim notification including the one when i saved my file into nvim-notify.

Here is my nvim-notify config (I'm using Lazy as my package manager)

return {
    "rcarriga/nvim-notify",
    config = function()
        local notify = require("notify")
        vim.opt.termguicolors = true
        vim.notify = notify
        vim.opt.showcmd = false
        vim.opt.cmdheight = 0
        vim.opt.laststatus = 2

        vim.api.nvim_create_autocmd('BufWritePost', {
            callback = function()
                local filename = vim.fn.expand('%:t') -- Get the full file path
                require('notify')('File saved successfully: ' .. filename, 'info', { title = 'Save Notification' })
            end,
        })

        local function override_echo()
            local original_echo = vim.api.nvim_echo
            vim.api.nvim_echo = function(messages, _, _)
                for _, message in ipairs(messages) do
                    notify(message[1], 'info', { title = 'Message' })
                end
            end
        end

        local function override_out_write()
            local original_out_write = vim.api.nvim_out_write
            vim.api.nvim_out_write = function(msg)
                notify(msg, 'info', { title = 'Output' })
            end
        end

        override_echo()
        override_out_write()

        notify.setup({
            background_colour = "#181926",
            fps = 60,
            render = "default",
            stages = "slide",
            timeout = 1700,
            max_height = 10,
            max_width = 80,
        })
    end
}

@MowlandCodes
Copy link
Author

It throws this error
image

@MowlandCodes
Copy link
Author

But when is removed the "create" parameter from inside the highlights.lua it runs normally

@MowlandCodes
Copy link
Author

image

@MowlandCodes MowlandCodes changed the title fixed including an invalid parameter on highlights.lua (erased 'creat… fixed including an invalid parameter on highlights.lua (erased "create" parameter) Dec 31, 2024
@wnineg
Copy link
Contributor

wnineg commented Dec 31, 2024

I just noticed my aged PR #298 was merged few days ago so I go back to this repo to check if it is causing any extra issues. The PR at the time didn't take backward compatibility into consideration. The "create" key was only introduced in neovim since v0.10.0 (neovim/neovim@8afb3a4) and I guess this issue was because of the older version?

Additionally, I noticed the PR also caused test failures since the neovim they were using was v0.5.1, while nvim_get_hl was only introduced since v0.9.0 (neovim/neovim@c0fe6c0).

So to fix this, we could either set the required neovim version to v0.10.0+, or mixin some api/version compatibility checking code.

Edit: The key api that was used for the performance fix was nvim__redraw, which was also introduced in v0.10.0+ (neovim/neovim@037ea6e)

@MowlandCodes
Copy link
Author

I guess is all about the version issues.... I'm using Neovim 0.9.5 from Ubuntu 24.04.1 LTS btw....

@MowlandCodes
Copy link
Author

the README.md from this repo is not including the required Neovim version though....

@wnineg
Copy link
Contributor

wnineg commented Dec 31, 2024

the README.md from this repo is not including the required Neovim version though....

I think its up to @rcarriga to decide if he wants to...

  1. explicitly document the minimal required version of neovim to v0.10.0+. (Needs to updated doc and workflow.)
  2. mix in some api version/compatibility branching code for different versions. (I could help to do so but I don't have time right now, maybe a couple days later.)
  3. create new branch for supporting older version. (Maybe not worth the headache for the current prerelease stage of neovim?)

@rcarriga
Copy link
Owner

rcarriga commented Jan 2, 2025

I won't be supporting anything other than nightly and the latest stable release, it's just too much of a pain to keep track of changes. I'll get around to updating the docs at some point, PRs welcome 😅

@MowlandCodes You can use the app image from https://github.com/neovim/neovim/releases if you want the latest without having to build manually, or if you want to stick with your OS's version you'll have to find the last compatible commit and pin it in your plugin manager

@rcarriga rcarriga closed this Jan 2, 2025
@MowlandCodes MowlandCodes deleted the fixed branch January 2, 2025 15:39
@MowlandCodes MowlandCodes restored the fixed branch January 2, 2025 15:39
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.

3 participants