fix: winfixbuf/current toggle bug from #1390

[cseickel]

This fixes the bug from #1390, but I have not tested it against winfixbuf yet. When you get a chance, can you review and test it? Thanks!

[pysan3]

No this will bring back the float + :vsplit README.md bug if I'm not mistaken.

[pysan3]

And as long as we have this bug, winfixbuf won't work as well. @cseickel

[pysan3]

BTW you've mentioned the wrong name lol. I have a 3 at the end ;)

@pysan3

[cseickel]

OK, we are going to have to explain why exactly this difference causes each bug. I'll try out your other fix later.

BTW you've mentioned the wrong name lol. I have a 3 at the end ;)

Oops 😊

[pysan3]

Prior to winfixbuf, when neo-tree is floating, it was guaranteed that there is at least one ground (non-floating) window where we can send the utils.open_file or stealing buffer (a buffer that was detected by the steal prevention mechanism).

However, when all ground windows are winfixbuf, we now need to make a new split before placing that buffer, just like the mechanism used for left/right neo-trees (when neo-tree is the only opened window).

This is why float + vsplit is now an official issue we need to take care of instead of ignoring it and saying oh you stupid user.

So, why does this happen in the first place and why does bufdelete wrapped inside a vim.schedule solve it?
Let me explain. (Forget about winfixbuf)

Section 1

:e LICENSE
:Neotree float

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT , bufname: neo-tree filesystem [1]

Section 2

:vs README.md

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT , bufname: neo-tree filesystem [1]
• winid: 1002, GROUND, bufname: README.md (on the right of window 1000)

Section 3

Since :vs README.md is a BufEnter event, our buffer_enter_event kicks in (setup/init.lua L150).
Keep in mind that the current window is 1002 so utils.is_floating() == false regardless of where we came from.
When you look inside that function, prior_type == "neo-tree" is true (around L217 in setup/init.lua) so vim.cmd("b#") is executed and the alt buffer ("#") is neo-tree's float so that's now in focus. Remember that a utils.open_file to open README.md is vim.schedule-ed right after that *1.

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT , bufname: neo-tree filesystem [1]
• winid: 1002, GROUND, bufname: neo-tree filesystem [1]

Section 4

At the same time, the operation is also a WinEnter event so setup/init > M.win_enter_event (around L264) is also executed. Inside that function, manager.close_all("float") is executed. This is what makes floating neo-tree automatically disappear when lost focus.
This will eventually end up at ui/renderer > M.close.
You can follow the conditions and eventually pcall(vim.api.nvim_win_close, state.winid, ... is called (L162) which closes the float window, good.

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT REMOVED
• winid: 1002, GROUND, bufname: neo-tree filesystem [1]

After that, pcall(vim.api.nvim_buf_delete, bufnr, ... (bufnr == state.bufnr) is called.
Since vim will close the window together when the current buffer closes, this operation closes window 1002 as you can see from the above list.

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT REMOVED
• winid: 1002, GROUND, bufname: neo-tree filesystem [1] REMOVED

Section 5

Do you remember *1: the utils.open_file scheduled in section 3? That is executed now since all actions from the previous loop is finished.
Basically utils.open_file will open the specified buffer with :b and as you can see, the current focused buffer is window 1000, as this is the only window left.

• winid: 1000, bufname: README.md, GROUND

This is why there's only a single window left even when we did :vs.

---

Why vim.schedule(bufdel) fixes that?

Let's rewind back to section 4.

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT REMOVED
• winid: 1002, GROUND, bufname: neo-tree filesystem [1]

We don't vim.api.nvim_buf_delete here so the situation remains like this, but we schedule to bufdel(neo-tree filesystem [1]).

section 5 starts before the scheduled bufdel so window 1002 is still alive and is the current window. So utils.open_file in section 5 is able to open the registered buffer in window 1002.

• winid: 1000, GROUND, bufname: LICENSE
• winid: 1001, FLOAT
• winid: 1002, GROUND, bufname: README.md

And then bufdel happens but since neo-tree filesystem [1] buffer is not associated to any window, it is killed on its own without falling together with a window, so we have the split remaining.

Just don't vim.cmd("b#") in the event if prior is float?

No if we don't do that, floating neo-tree becomes steal-able. Meaning that :e README.md on a floating window will open README.md inside that float, stealing neo-tree's window.

More Carefully Constructed If Conditions in buffer_enter_event?

Go ahead ;) And be sure there's no edge cases for your approach.
TBH my rewrite deals with this in a different way and doesn't have such issue from the start, so I just don't have the motivation.

[pysan3]

@cseickel

[cseickel]

Thanks for the detailed run down @pysan3. As far as I can see, the code in this PR still handles the float + winfixbuf window situation perfectly and creates a split when needed. You may not have noticed that it still has the scheduled buffer delete, it's just that decision on whether or not that buffer should be deleted happens inline instead of being scheduled.

I'm going to merge this because it seems to work as is.

[pysan3]

No. It doesn't. You definitely missed something.

[cseickel]

No. It doesn't. You definitely missed something.

Huh, can you give me a numbered list of instructions how to recreate the bug?

[pysan3]

@cseickel #1390 (comment)

I didn't say anything about winfixbuf. It is a bug even from before the PR.

[cseickel]

@cseickel #1390 (comment)

I didn't say anything about winfixbuf. It is a bug even from before the PR.

I'm lost.

[pysan3]

@cseickel Please do this and show me a screen record for both your fix and my PR. Sorry to bother your time.

• :e <file-A>
• :Neotree float
  • Don't close this
• :vsplit <file-B>

!Don't close this was just a comment, meaning to literally press : and start writing vsplit while floating neo-tree is in focus.

record.mp4

Lazy won't close automatically so there's a difference, but you can see that the new buffer should be created as a new split in the background.

[cseickel]

Yeah, I have the same results when doing that particular test. That is such an oddball thing to do though that it's not worth causing bugs in the normal expected workflow in order to handle this really wacky edge case.

[pysan3]

It is a real bug!!

Please read my comment #1408 (comment).
I explain why we now need to deal with this bug seriously due to winfixbuf in the first few paragraphs.

[cseickel]

Honestly, when I read that the first time I was assuming you meant to issue an open split command with a Neotree mapping, because why would anyone actually issue :vsplit <filename> while the Neotree float is open?.

Logic would dictate that if you opened a file explorer, you would open the file from that file explorer. It's hard to imagine why you would open a floating file picker and then manually issue a command to open a file while that floating window is still open.

[pysan3]

However, when all ground windows are winfixbuf, we now need to make a new split before placing that buffer, just like the mechanism used for left/right neo-trees (when neo-tree is the only opened window).

This is why float + vsplit is now an official issue we need to take care of instead of ignoring it and saying oh you stupid user.

As I said here, when there's no ground window available because all are winfixbuf, we need to make a new split before opening the buffer to make a place where the buffer can be opened.

This is exactly the same as calling :vs README.md while the floating window is open.

So if float + :vs doesn't work, winfixbuf support is broken as well.
It is easier and more obvious to run :vs than preparing a setup with winfixbuf so that's what I used for testing.

Does this make sense?

• :e release.sh
• :set winfixbuf
• :Neotree float
• focus README.md
• <CR> (open the file)

Now, because the ground window (where release.sh lives) is a winfixbuf, README.md cannot be opened in that window. Since that's the only available option, we need to make a blank split next to release.sh instead to open README.md.

So we need to use :vs README.md instead to make a new split window next to release.sh before closing neo-tree float.

[cseickel]

However, when all ground windows are winfixbuf, we now need to make a new split before placing that buffer, just like the mechanism used for left/right neo-trees (when neo-tree is the only opened window).
This is why float + vsplit is now an official issue we need to take care of instead of ignoring it and saying oh you stupid user.

As I said here, when there's no ground window available because all are winfixbuf, we need to make a new split before opening the buffer to make a place where the buffer can be opened.

This is exactly the same as calling :vs README.md while the floating window is open.

Nope, I don't agree with that at all.

So if float + :vs doesn't work, winfixbuf support is broken as well. It is easier and more obvious to run :vs than preparing a setup with winfixbuf so that's what I used for testing.

Nope.

Does this make sense?

• :e release.sh
• :set winfixbuf
• :Neotree float
• focus README.md
• <CR> (open the file)

Now, because the ground window (where release.sh lives) is a winfixbuf, README.md cannot be opened in that window. Since that's the only available option, we need to make a blank split next to release.sh instead to open README.md.

Those instructions do make sense, and that actually works fine in main right now, even though issuing a :vsplit command does not make a split when neotree float is open.

regardless, I think #1406 is a good improvement that handles everything so I guess the discussion is resolved.

[pysan3]

However, when all ground windows are winfixbuf, we now need to make a new split before placing that buffer, just like the mechanism used for left/right neo-trees (when neo-tree is the only opened window).
This is why float + vsplit is now an official issue we need to take care of instead of ignoring it and saying oh you stupid user.

As I said here, when there's no ground window available because all are winfixbuf, we need to make a new split before opening the buffer to make a place where the buffer can be opened.
This is exactly the same as calling :vs README.md while the floating window is open.

Nope, I don't agree with that at all.

Why? You do deal with this problem with left/right windows.

• :e release.sh
• :Neotree left
• <C-w>l (go back to release.sh)
• <C-w>q (delete that window)
• now neo-tree covers the whole screen
• focus README.md
• <CR>

when there's no ground window available because all are winfixbuf except for neo-tree, we need to make a new split before opening the buffer to make a place where the buffer can be opened.
This is exactly the same as calling :vs README.md while the floating side window is open.

I think I'm saying the exact same thing? And why do you deal with this case but not for the floating window? I don't agree with this inconsistency.

[pysan3]

@cseickel

[cseickel]

I'm sorry I should have explained better. What I meant was that I disagree with the assertion that manually issuing a :vsplit command is somehow equivalent to using the neotree float window to open a file. The process that it goes through to end up creating a new split when it is needed is completely different and the manual split command was not a valid test case.

[pysan3]

The process that it goes through to end up creating a new split when it is needed is completely different and the manual split command was not a valid test case.

Could you explain the difference?

Manual split works -> but neo-tree doesn't may happen but I don't think for the other way around.
If manual split doesn't work, neo-tree's creating new split code will also always not work as well.

So manual splitting should be a valid test case.

[cseickel]

I know empirically that it is not the same because during testing of the various states of changes, using Neotree to open a file either normally or in vsplit would work just fine while your :vsplit <filename> did not work as expected.

Without that evidence though, I know that it opening a file from neotree does not issue a vsplit while the floating window is in focus. It switches to the last used window first then sees if it is usable. If not, it keeps switching windows until it finds one it likes and then issues the command. There are more nuances if the only open window is a neo-tree window that is supposed to be a sidebar.