Bekaboo / dropbar.nvim

IDE-like breadcrumbs, out of the box
GNU General Public License v3.0
990 stars 23 forks source link

fix: fatal with invalid buffer #114

Closed Parsifa1 closed 10 months ago

Parsifa1 commented 10 months ago

image

Before executing buf_get_lines, add a check to see if the buf_id is valid.

Bekaboo commented 10 months ago

@Parsifa1 As @willothy suggests, please tell us how to reproduce this issue.

Parsifa1 commented 10 months ago

It seems the real problem comes from this plugin it provides a feature to Resume previous session. When there is a markdown buffer, it will be resumed by that plugin, then the issue appeared. In a word, this problem does not come from the functionality of dropbar and Vanilla neovim. but there's nothing wrong in this "buffer valid check",right?

Bekaboo commented 10 months ago

Thanks, if the function relies on the validity of buf, better to check it at the very beginning instead of in the middle of the function to avoid unnecessary computation.

Parsifa1 commented 10 months ago

This is my first time making some pr stuff, maybe a little bad, ...Thank you anyway!

Bekaboo commented 10 months ago

This is my first time making some pr stuff, maybe a little bad

It is not bad at all. Thanks for reporting this issue and coming up with the PR, it helps us making dropbar.nvim more stable. Please keep goinig!

willothy commented 10 months ago

This is my first time making some pr stuff, maybe a little bad

It is not bad at all. Thanks for reporting this issue and coming up with the PR, it helps us making dropbar.nvim more stable. Please keep goinig!

+1 to this, it's not bad in any way! Thank you for the PR, it is much appreciated even if it didn't end up being merged.

If you're ever interested in making another PR here I'd be happy to guide that process / answer any questions about the codebase :)

willothy commented 10 months ago

Thanks, if the function relies on the validity of buf, better to check it at the very beginning instead of in the middle of the function to avoid unnecessary computation.

I'm still confused as to why this was happening in the first place though... Maybe we should make an issue for this so the root cause can be investigated, even if it's not technically an issue anymore?

I don't think the :del() method should be triggering the parsing of a buffer, right? It's likely something in that method is invoking the __index metamethod which parses the buffer. Maybe the check for self.buf? If so, using rawget could solve this issue without any kind of buffer validity check.

Bekaboo commented 10 months ago

I'm still confused as to why this was happening in the first place though... Maybe we should make an issue for this so the root cause can be investigated, even if it's not technically an issue anymore?

Yeah, but this depends on @Parsifa1 to provide minimal production steps since I haven't had the issue.

I don't think the :del() method should be triggering the parsing of a buffer, right?

You are right, :del() should not require any sources.

It's likely something in that method is invoking the __index metamethod which parses the buffer. Maybe the check for self.buf? If so, using rawget could solve this issue without any kind of buffer validity check.

I am not quite sure what is your solution here. If the __index() method of _G.dropbar.bars() creates a new dropbar instance AND (1) that instance has markdown as its source AND (2) the dropbar is attached to a markdown buffer, then the get_symbols() function of the markdown source will be called. If the buffer is unloaded the attached bar should be deleted by dropbar_t:del().

willothy commented 10 months ago

I'll try to reproduce based on their description, since I already have project.nvim installed.

I am not quite sure what is your solution here.

You're right, looking back over the code I don't think I was correct. I'll try to reproduce with my config, but if that doesn't work we can just wait for repro steps.

Edit: oops, wrong project.nvim lol I use a different one with essentially the same name. I'll install it and see if I can repro anyways though.

Parsifa1 commented 10 months ago

it's this plugin https://github.com/coffebar/neovim-project

As for the minimum configuration
for neovim-project.

return {
    "coffebar/neovim-project",
    -- cmd = "Telescope",
    event = "VeryLazy",
    opts = {
        last_session_on_startup = false,
        projects = {
               -- define project roots
        },
    },
    init = function()
        -- enable saving the state of plugins in the session
        vim.opt.sessionoptions:append("globals") -- save global variables that start with an uppercase letter and contain at least one lowercase letter.
    end,
    dependencies = {
        { "nvim-lua/plenary.nvim" },
        { "nvim-telescope/telescope.nvim" },
        { "Shatur/neovim-session-manager" },
    },
}

for dropbar.nvim

-- local custom = require "custom"

return {
    "Bekaboo/dropbar.nvim",
    -- "Parsifa1/dropbar.nvim",
    dependencies = { 'nvim-telescope/telescope-fzf-native.nvim' },
    event = "VeryLazy",
    opts = {
        general = {
            enable = function(buf, win)
                return not vim.api.nvim_win_get_config(win).zindex
                    and vim.bo[buf].buftype == ""
                    and vim.api.nvim_buf_get_name(buf) ~= ""
                    and not vim.wo[win].diff
                    and vim.bo[buf].filetype ~= "toggleterm"
            end,
        },
    },
}

and the step is :

  1. add at least two project in the plugin
  2. ensure that there are at least one project with a markdown in it
  3. use :Telescope neovim-project discover, and switch some times
  4. then the error may occur
willothy commented 10 months ago

Thanks @Parsifa1! What version of Neovim are you using?

Parsifa1 commented 10 months ago

Thanks @Parsifa1! What version of Neovim are you using?

nightly