nvim-lua / plenary.nvim

plenary: full; complete; entire; absolute; unqualified. All the lua functions I don't want to write twice.
MIT License
2.53k stars 270 forks source link

Plenary Callback issue in dependent plugin #588

Closed augustdolan closed 1 month ago

augustdolan commented 1 month ago

Hey all,

If there is documentation to write better issues for y'all, feel free to send that along and I can clean up this issue. In the meantime, I've been directed to open an issue here as opposed to with https://github.com/CopilotC-Nvim/CopilotChat.nvim since the issue I am encountering is from plenary.

You can find the issue thread here: https://github.com/CopilotC-Nvim/CopilotChat.nvim/issues/325#issue-2306083916

Surfacing the issue to this thread below:

When calling this plugin's main function :CopilotChat <input> the following is received:


stack traceback:
    [C]: in function 'has'
    ....local/share/nvim/lazy/plenary.nvim/lua/plenary/curl.lua:41: in function 'headers'
    ....local/share/nvim/lazy/plenary.nvim/lua/plenary/curl.lua:241: in function 'request'
    ....local/share/nvim/lazy/plenary.nvim/lua/plenary/curl.lua:277: in function 'post'
    ...e/nvim/lazy/CopilotChat.nvim/lua/CopilotChat/copilot.lua:408: in function 'on_done'
    ...e/nvim/lazy/CopilotChat.nvim/lua/CopilotChat/copilot.lua:319: in function '_user_on_exit'
    .../.local/share/nvim/lazy/plenary.nvim/lua/plenary/job.lua:241: in function '_shutdown'
    .../.local/share/nvim/lazy/plenary.nvim/lua/plenary/job.lua:48: in function <.../.local/share/nvim/lazy/plenary.nvim/lua/plenary/job.lua:39>```

A collaborator on that thread suggests it may be a regression issue, but I wouldn't know where to start to check, and that individual seemed to be making an intuition call.

Please let me know what, if anything, I can do to help resolve this! 
Conni2461 commented 1 month ago

fixed by https://github.com/nvim-lua/plenary.nvim/pull/589

my bad for introducing this regression, didnt think that anyone is using the module in an fast event. could you test that PR?

augustdolan commented 1 month ago

@Conni2461 thanks so much for getting back so quickly. I would love to test the PR. However, this is my first time foraying into version control with Lazy. Is it possible for your to provide some guidance? I've tried:

  {
    "CopilotC-Nvim/CopilotChat.nvim",
    branch = "canary",
    dependencies = {
      { "zbirenbaum/copilot.lua" },                                       -- or github/copilot.vim
      { "Conni2461/plenary.nvim", branch = "fix/in_fast_loop" }, -- for curl, log wrapper
    },
    opts = {
      debug = true, -- Enable debugging
      -- See Configuration section for rest
    },
    -- See Commands section for default commands if you want to lazy load on them
  },
}

as well as

  {
    "CopilotC-Nvim/CopilotChat.nvim",
    branch = "canary",
    dependencies = {
      { "zbirenbaum/copilot.lua" },
      { "nvim-lua/plenary.nvim", branch = "Conni2461:fix/in_fast_loop" }, 
    },
    opts = {
      debug = true,
    },
  },
}

to try and depend on your branch, but both options give me a refspec error.

I've been just calling nvim-lua/plenary.nvimas a dependency for a few plugins and tried simply changing the dependency on that plugin, so maybe its a namespace issue somehow? Sorry I am not able to test it yet. Later on I can try to install plenary directly and do the same versioning there if no one has any insight.

Thanks so much for your help, apologies again

Conni2461 commented 1 month ago

lazy.nvim just clones the repo in .local/share/nvim/lazy/plenary.nvim it should be enough to open that directory and manually checkout that branch. even with gh pr checkout 589 and after that switch back to the previous branch and later updating with :Lazy. you dont need to get fancy

augustdolan commented 1 month ago

Thanks Conni, that made good, plain sense and put my head on straight about the actual relationship Lazy has with plugins. I am no longer receiving the error I opened this issue for when using your branch.

Thanks so much, and again I can confirm this fixes the plenary issue.

Conni2461 commented 1 month ago

thanks for testing

tris203 commented 1 month ago

Apologies for the regression @Conni2461

it was my PR that introduced it! I don't really understand what was going wrong, was it the inner function calling before the inline function could return? Id like to understand what I could have done better, and why.

Apologies for the head-scratching!

Conni2461 commented 1 month ago

Apologies for the regression

don't worry, it can happen. The problem was that your flatten method was called in a fast lua loop, and contained a vim.fn.has function call with is not api-fast and we are not allowed to call that at this point in time. The solution now is to do the vim.fn.has check on module load, which is always safe to do and then either set compat.flatten to the old or the new way of doing things, the version value will also never change while nvim is open, so its safe to do that and cache the result like that

you can read more here :help api-fast and :help vim.in_fast_event()

7adidaz commented 1 month ago

@Conni2461 thanks for the fix:)