Fix : "attempt to index a number value"

[ls-devs]

Fix "attempt to index a number value" on latest neovim build.

[dsully]

This fixes nvim-notify on nightly for me as well.

[GitMurf]

what is best way to apply this until the repo maintainer merges the PR? is the only real option to manually go replaces the file in my local installation? Thanks!

[ls-devs]

what is best way to apply this until the repo maintainer merges the PR? is the only real option to manually go replaces the file in my local installation? Thanks!

You can just change rcarriga/nvim-notify by ls-devs/nvim-notify in your nvim config where you call the plugin I guess ?

[dpetka2001]

@ls-devs

This PR is incomplete. It doesn't work for Neovim 0.9.5 (latest stable). If it gets merged like this, users who use 0.9.5 will have notifications broken. Notifications work fine with the current state of lua/notify/stages/util.lua file in 0.9.5. I would suggest adding guards against the version of Neovim being used. Something like

if vim.fn.has("nvim-0.10") == 1 then
  local exists, cur_win_conf = util.get_win_config(win)
else
  local exists, cur_win_conf = pcall(vim.api.nvim_win_get_config, win)
end

The same should be done for the rest of the changes made in this PR.

Maybe there is a better way to do this, but I believe you get the idea.

[ls-devs]

@dpetka2001 Thank you for pointing it out. For now I'll commit with guards everywhere it's needed. If anyone have an idea of how to do it more clean let us know.

[ls-devs]

@dpetka2001 Let me know if this commit fix your issue. But, tbh, I don't know if this is the right way to do it. There must be a cleaner way to achieve this without using if ... else everywhere in the code.

[dpetka2001]

Did you test it? For me it doesn't solve the issue. I created a branch in my forked repo that appears to solve it for both nightly and stable. Here you can see the commit with the differences. Please feel free to incorporate the changes. In your last commit you appear to make changes in the window_intervals function as well, but I believe that's not needed. Only the slot_after_previous function needs changes.

[xfzv]

@dpetka2001 I've just tried your fork with Neovim nightly and the issue persists while the original PR from @ls-devs fixes it.

$ nvim -v
NVIM v0.10.0-dev-2361+ga376d979b-dirty

  nvim-notify 0.18ms  noice.nvim
        dir    /home/xfzv/.local/share/nvim/lazy/nvim-notify
        url    https://github.com/dpetka2001/nvim-notify
        branch master
        commit 80b67b2
        readme README.md
        help   |nvim-notify.txt|

  nvim-notify 0.19ms  noice.nvim
        dir    /home/xfzv/.local/share/nvim/lazy/nvim-notify
        url    https://github.com/ls-devs/nvim-notify
        branch master
        commit 9a6ca0c
        readme README.md
        help   |nvim-notify.txt|

[dpetka2001]

@xfzv You're using the master branch which is the initial state of the fork. Use the fix/index_value branch like so

  {
    "dpetka2001/nvim-notify",
    branch = "fix/index_value",
  },

[ls-devs]

Did you test it? For me it doesn't solve the issue. I created a branch in my forked repo that appears to solve it for both nightly and stable. Here you can see the commit with the differences. Please feel free to incorporate the changes. In your last commit you appear to make changes in the window_intervals function as well, but I believe that's not needed. Only the slot_after_previous function needs changes.

I tested it. But was in a hurry so I might no had used the right nvim version. I rectified with your changes. Please tell me if it works because its working for me.

[latipun7]

The last change works for me too 👍

[dpetka2001]

@ls-devs Yes the latest changes solve the problem for both nightly and stable from the testing that I did. I would like to suggest that other people test it both on nightly and stable if possible, so that we haven't overlooked any edge case.

[xfzv]

@xfzv You're using the master branch which is the initial state of the fork. Use the fix/index_value branch like so

  {
    "dpetka2001/nvim-notify",
    branch = "fix/index_value",
  },

My bad, I didn't pay attention to the branches. Can confirm the issue is also fixed using the fix/index_value branch:

  nvim-notify 0.19ms  noice.nvim
        dir    /home/xfzv/.local/share/nvim/lazy/nvim-notify
        url    https://github.com/dpetka2001/nvim-notify
        branch fix/index_value
        commit d590b4d
        readme README.md
        help   |nvim-notify.txt|

[dpetka2001]

@xfzv Please continue to use the branch of @ls-devs which is also associated with this PR. I will delete mine, since there's no purpose existing any more. I only created it to solve the problem for Neovim 0.9.5 which the initial state of the PR didn't take into account.

[iwansyahp]

in case there is someone who use LazyVim and encounter same issue, you can use proposed solution here by creating a file under ~/.config/nvim/lua/plugins

nvim-notify.lua

return {
  {
    "ls-devs/nvim-notify",
    keys = {
      {
        "<leader>un",
        function()
          require("notify").dismiss({ silent = true, pending = true })
        end,
        desc = "Dismiss all Notifications",
      },
    },
    opts = {
      timeout = 3000,
      max_height = function()
        return math.floor(vim.o.lines * 0.75)
      end,
      max_width = function()
        return math.floor(vim.o.columns * 0.75)
      end,
      on_open = function(win)
        vim.api.nvim_win_set_config(win, { zindex = 100 })
      end,
    },
    init = function()
      -- when noice is not enabled, install notify on VeryLazy
      if not require("lazyvim.util").has("noice.nvim") then
        require("lazyvim.util").on_very_lazy(function()
          vim.notify = require("notify")
        end)
      end
    end,
  },
}

[dpetka2001]

@iwansyahp I believe you don't need all this. With just

return {
  {
    "ls-devs/nvim-notify",
    branch = "fix/fix_index_value",
  },
}

You should be ok. The opts from the LazyVim part should still be merged with yours.

[kawre]

Using a separate function for extracting slots would be easier to maintain for sure

local function get_slot(conf, _key)
  return vim.version().minor < 10 and conf[_key][false] or conf[_key]
end

then use it like this

get_slot(win_confs[open_win], key)

get_slot(cur_win_conf, key)

[dpetka2001]

@ls-devs I like @kawre's idea and seems more maintainable as well. It's up to you to make the final decision to incorporate it or not.

[mehalter]

Would it also be safer to do

local function get_slot(conf, _key)
  return vim.fn.has "nvim-0.10" == 1 and conf[_key] or conf[_key][false]
end

That way it protects against whenever 1.0 comes out and the minor version goes below 10 again. Not that that's happening any time soon, but just for down the line.

[ls-devs]

@mehalter You are totally true, I didn't thought about this kind of issue. Will correct it tomorrow. Thank you for pointing it out.

[rcarriga]

Thanks for the PR! :smile: I think the simplest solution here would be just switch both calls to nvim_win_get_config with util.get_win_config and then remove all [false] indexes (as if on >= 0.10.0). Am I missing something?

[dpetka2001]

Sorry for the wrong alert. I was under the assumption that when I make a suggestion I have to press approve for the PR to not be blocked. Still getting comfortable with Github and programming. Please disregard the approved notification by me.

[ls-devs]

Thanks for the PR! 😄 I think the simplest solution here would be just switch both calls to nvim_win_get_config with util.get_win_config and then remove all [false] indexes (as if on >= 0.10.0). Am I missing something?

Yes that's was in my mind. I'll make the changes a fast as I can tomorrow so it can be reviewed and tested 👍🏼

[ls-devs]

@dpetka2001 , @rcarriga

So I created get_safe_slot function to handle all the cases where we need to remove [false] or not. I also replaced nvim_win_get_config with util.get_win_config. Would you please test it and tell me if it works for you ?

[wookayin]

I don't think we need such an utility --- needing to use such a field accessor on every single callsite would be unnecessarily complicated and error-prone. We can instead make the win_conf table (returned by util.get_win_config) always contain a float ("number", not "lua-special-tbl" { [false] = ... [true] = ... }) for each of the relevant fields, no matter which nvim version is used (i.e. write a compat wrapper for nvim < 0.10). In this way one can access the number always with win_conf[key].

[ls-devs]

@wookayin Your idea seems to be the better solution for this issue. Would you please provide an example of how you would implement this ? I think I don't have enough knowledge about neovim to implement this alone.

[dpetka2001]

@ls-devs Based on what @rcarriga said, util.get_win_config seems to inherently manage this thing, so I don't believe you need the extra slot thing you added. I created an example branch based on what @rcarriga proposed. Take a look here. I tested it on both stable and nightly and it seems to work on my end.

[ls-devs]

@ls-devs Based on what @rcarriga said, util.get_win_config seems to inherently manage this thing, so I don't believe you need the extra slot thing you added. I created an example branch based on what @rcarriga proposed. Take a look here. I tested it on both stable and nightly and it seems to work on my end.

Sorry, I misunderstood what he said ! I thought both was necessary. I'll make a quickfix and push it.

[dpetka2001]

Please also test it on your end after your local changes, just to avoid something that I might have missed.

[ls-devs]

@dpetka2001 Can confirm it's working on both nightly and stable. Sorry for the useless PRs and my misunderstanding of what @rcarriga said.

[dpetka2001]

No worries, I had also suggested in a previous comment to use the default Neovim API call instead of util.get_win_config, because I hadn't looked carefully into what it does until rcarriga proposed it. Then I looked again and realised that all these version checks were unnecessary, since util.get_win_config already makes that transformation for us and returns the window handler just like it's being returned from the Neovim API call in nightly as well.

[iwansyahp]

thanks @dpetka2001 for your answer here, I am new to vim.

[rcarriga]

Awesome thanks for all the help everyone :grin: