[Bug]: Offset only uses one window #789

FalcoGer · 2023-07-13T12:31:20Z

Is there an existing issue for this?

I have searched the existing issues

What happened?

When multiple offsets are defined, then opening a new window will only apply the offset configuration to one window, usually the leftmost one or the last one opened. It behaves oddly.

For example when having an offset for filetype "help" and an offset for filetype "NvimTree", then opening NvimTree will produce the correct behavior. If you then however open the help with :vertical help, the help window gets the offset configuration applied but the nvim-tree gets it's text deleted.

To reproduce:

Open nvim and only have one buffer open that doesn't have an offset setting for it's filetype
Open nvim-tree
Open vertical help
Notice help doesn't get the title and instead the bufferline starts over the help window
Close nvim-tree
Notice that help now gets the offset applied where before it did not

Windows that open on the right, such as tagbar, also behave oddly when opening and closing them mixed with opening and closing multiple windows that open on the left, such as undotree.

What did you expect to happen?

Vertically split windows always get their header text and all the offsets are added for the bufferline, except of course for the offsets for windows on the right, which should be handled separately

Config

-- ...
offsets = {
    {
        filetype = "NvimTree",
        text = function()
            return "NVIMTREE TEXT"
            -- return vim.fn.getcwd()
        end,
        -- highlight = "Directory",
        text_align = "left",
        separator = true
    },
    {
        filetype = "NERDTree",
        text = function()
            return vim.fn.getcwd()
        end,
        highlight = "Directory",
        text_align = "left",
        separator = true
    },
    {
        filetype = "undotree",
        text = "Undo Tree",
        text_align = "left",
        separator = true
    },
    {
        filetype = "diff",
        text = "Diff",
        text_align = "left",
        separator = true
    },
    {
        filetype = "tagbar",
        text = "Tags",
        text_align = "left",
        separator = true
    },
    {
        filetype = "help",
        text = "Help",
        text_align = "left",
        separator = true
    }
},
-- ...

Additional Information

N/A

commit

No response

The text was updated successfully, but these errors were encountered:

akinsho · 2023-07-13T13:26:43Z

Hi @FalcoGer,

So this is an issue I've discussed many times over the course of the history of this feature leading all the way back to when I initially added it. It is absolutely not a full proof feature that covers all use cases perfectly. The main object was for it to be good enough for 90% of people's use cases e.g. having a file explorer open etc.

The main difficulty with getting this perfectly right is that it works based on getting the window layout for nvim which comes in the form of nested array which describe the layout of neovim's windows. These array can be in a a lot of different configurations some of which apply to an offset and many more which don't. The offset logic just tries to find the ones that do apply and provide offsets for those.

This may or may not be easily solvable by someone taking a look and having a go.
I'll explain my position on this. This feature to my mind is a good is better than perfect sort of thing IMO. It won't hit all cases and it's a pain to try to make it do that since as I mentioned I personally believe 90% of the use case is a single file explorer to the left.

When I implemented the feature I explained to the user who requested it at the time that it was going to be a good enough type solution not a perfect solution. I've also described this in many issues about this involving undotree et al.

I'm a very big believer in community involvement in open source projects, I think if this bothers you you are very welcome to help contribute a fix and I can help guide that. This bug is not ideal but frankly the time it might take me to dig into this or how to fit this into the many many other issues and bugs across multiple plugins I have to deal with means this is not something I'm likely to personally prioritise as it doesn't affect me in my daily work at all.

TLDR: PR welcome please 🙏🏾 otherwise I will leave this open for a week or two (or till I remember to check back) otherwise I will close this out.

FalcoGer · 2023-07-13T14:32:59Z

The problem is that I'm not very good at lua or vimscript.

However it seems the problem appears to be solvable by examining wininfo(), which has the fields wincol and winrow. Since you only care about the windows in the top, because that's where the bufferline goes, you can check all windows with winrow == 2 (bufferline is in row 1) and apply the offset thing where it matches, and then place the bufferlist in the first available gap or the largest gap or something.

For example

akinsho · 2023-07-13T15:19:28Z

The problem is that I'm not very good at lua or vimscript

@FalcoGer no vimscript knowledge is really required at all since there is not really any vimscript in this plugin and tbh lua is a very simple language. I think if you are able to configure your editor with it/know literally any programming languages that makes lua kind of trivial for this level of change.

Thanks for digging into potential solutions. Seems this mechanism could maybe simplify things massively by just doing something like

local offsets = {}
for _, win in ipairs(vim.fn.getwininfo()) do
	if win.winrow == 2 and (wincol == 1 or wincol == max) then
		table.insert(offsets, win.id)
	end
end

or something like this.

You could just remove this logic and the usage

bufferline.nvim/lua/bufferline/offset.lua

Lines 108 to 121 in cd27a52

    
           local function is_offset_section(windows, offset) 
        
             local wins = { windows[1] } 
        
             if #windows > 1 then wins[#wins + 1] = windows[#windows] end 
        
             for idx, win in ipairs(wins) do 
        
               local valid_layout, win_id = is_valid_layout(win) 
        
               if valid_layout then 
        
                 local buf = api.nvim_win_get_buf(win_id) 
        
                 local valid = buf and vim.bo[buf].filetype == offset.filetype 
        
                 local is_left = idx == 1 
        
                 if valid then return valid, win_id, is_left end 
        
               end 
        
             end 
        
             return false, nil, nil 
        
           end

vim.fn.layout and just add a new function like is_offset_win(offset)
and do a check similar to the above to return true or false

I think if you give it a try it'll be simpler than you thing probably and you can post questions here.

FalcoGer · 2023-07-13T17:06:05Z

It might not actually be winrow 2. probably best to get the minimum first. For example vim-airline has an option to be displayed at the top. not sure if it then goes above or below bufferline. Either way it will probably push the windows down into row 3.

fixes #789

akinsho · 2023-08-02T09:00:26Z

@FalcoGer gave this a try in #800 don't think the vim airline thing is a real issues ince the only thing that can actually be in the top bar is the tabline and airline is just using it for the statusline in that case

fixes #789

toh995 · 2024-01-09T05:03:20Z

Hi @akinsho thanks for your work on this. I noticed the PR is still unmerged - is there anything blocking the merge? Maybe I can help?

akinsho · 2024-01-15T18:27:38Z

There are still bugs with that pr and I don't have time to work on it now

FalcoGer added the bug Something isn't working label Jul 13, 2023

akinsho added a commit that referenced this issue Aug 2, 2023

refactor(offsets): use wininfo to calculate offset

aa0b1d7

fixes #789

akinsho linked a pull request Aug 2, 2023 that will close this issue

refactor(offsets): use wininfo to calculate offset #800

Draft

akinsho added a commit that referenced this issue Aug 2, 2023

refactor(offsets): use wininfo to calculate offset

13a959d

fixes #789

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: Offset only uses one window #789

[Bug]: Offset only uses one window #789

FalcoGer commented Jul 13, 2023

akinsho commented Jul 13, 2023

FalcoGer commented Jul 13, 2023 •

edited

Loading

akinsho commented Jul 13, 2023

FalcoGer commented Jul 13, 2023

akinsho commented Aug 2, 2023

toh995 commented Jan 9, 2024

akinsho commented Jan 15, 2024 •

edited

Loading

[Bug]: Offset only uses one window #789

[Bug]: Offset only uses one window #789

Comments

FalcoGer commented Jul 13, 2023

Is there an existing issue for this?

What happened?

What did you expect to happen?

Config

Additional Information

commit

akinsho commented Jul 13, 2023

FalcoGer commented Jul 13, 2023 • edited Loading

akinsho commented Jul 13, 2023

FalcoGer commented Jul 13, 2023

akinsho commented Aug 2, 2023

toh995 commented Jan 9, 2024

akinsho commented Jan 15, 2024 • edited Loading

FalcoGer commented Jul 13, 2023 •

edited

Loading

akinsho commented Jan 15, 2024 •

edited

Loading