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

fix: checking for winid value before to evoke it #203

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

victorfds
Copy link
Contributor

@victorfds victorfds commented Jan 19, 2024

To handle the error below:
240119_11h40m13s_screenshot

I noticed in the original code that the winid property was invoked (with ()) even when it was nil.
The refined code incorporates a validation step to ensure that the function is called exclusively when a valid value is present.

@j-hui
Copy link
Owner

j-hui commented Feb 3, 2024

Sorry for not getting to this sooner. But it's odd to me that this is necessary, since this function is quite clearly documented in the API: https://github.com/nvim-tree/nvim-tree.lua/blob/f39f7b6fcd3865ac2146de4cb4045286308f2935/doc/nvim-tree-lua.txt#L1720-L1730

Can you please let me know which version of nvim-tree you are using?

@j-hui j-hui added bug Something isn't working crash Plugin throws a runtime error labels Feb 3, 2024
@victorfds
Copy link
Contributor Author

Thank you for answering me.
Sure!
My nvim-tree version is 0.99.

@j-hui
Copy link
Owner

j-hui commented Feb 4, 2024

Can you please give me a minimal nvim config that reproduces this issue? I'm not able to reproduce it myself, using nvim-tree with tag = "v0.99".

I don't see any reason for the winid function to be missing, and I'd rather not add spurious checks where unnecessary because it clutters the code.

@victorfds
Copy link
Contributor Author

Sure.
I installed LunarVim.
After that, copied the Rust settings config.lua from https://github.com/LunarVim/starter.lvim/tree/rust-ide to my lvim custom file.
Then, the bug appeared.

@j-hui
Copy link
Owner

j-hui commented Feb 5, 2024

Is the problem unique to using LunarVim? If not, can you please provide a minimal config that does not involve installing a whole Neovim distro?

@j-hui
Copy link
Owner

j-hui commented Feb 5, 2024

I ended up digging into this myself, and found the issue: LunarVim does not use rolling plugin releases, and instead uses the commit hashes from its snapshots.json file (which essentially acts as a lock file). On v1.3.0 of LunarVim, that file is 10 months out of date at this point, and points to commit nvim-tree/nvim-tree.lua@bb375fb of nvim-tree.lua.

So in fact, you are probably not on v0.99 of nvim-tree, which was released on Dec 31, 2023. (I say "probably" because I find it odd that nvim-tree does not have any other releases aside from v0.99, so perhaps I misunderstand their versioning model and "previously released" v0.99 and later moved the tag forward on NYE.)

Since this means there are users of outdated nvim-tree, we should indeed check for whether winid is defined. However, doing so in each callback is really unnecessary---instead, we can simply not register the callback in the first place if the function doesn't exist. I'll go ahead and update the PR and merge it.

@gegoune
Copy link

gegoune commented Feb 5, 2024

You are correct, @j-hui, 0.99 is the only ever released version of nvim-tree.

@j-hui
Copy link
Owner

j-hui commented Feb 5, 2024

@victorfds please pull and make sure that this fixes your issue. If so, I'll go ahead and merge.

@victorfds
Copy link
Contributor Author

victorfds commented Feb 5, 2024

It works perfectly!

@j-hui j-hui merged commit 53d5b79 into j-hui:main Feb 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash Plugin throws a runtime error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants