onivim / oni

Oni: Modern Modal Editing - powered by Neovim
https://www.onivim.io
MIT License
11.36k stars 300 forks source link

Tabs no longer close correctly in buffer mode on latest #2639

Closed g-erson closed 5 years ago

g-erson commented 5 years ago

Oni Version: Built from latest commit on master, commit hash 2ae1224adf0659003355a62582641f8b45d37399

Neovim Version (Linux only): 0.2.2. Operating System: Ubuntu 18.04

Issue: :close, :quit no longer (visually) quit tabs. Instead the tab's top bar still remains but am unable to switch to it with gt/gT.

Expected behavior: The relevant tab disappears

Actual behavior: It remains

Steps to reproduce: This could also be related to my tabs mode, which is currently set to 'buffer'.

akinsho commented 5 years ago

@GoAwayPeter I've definitely noted this two thanks for actually raising the issue though not entirely clear on what changed between versions 🤔

CrossR commented 5 years ago

I've not noticed this (I have noticed the selection occasionally being off, ie tab 2 being highlighted when tab 6 is active), but they close correctly.

I'll assume this is a "buffer" specific issue, since I use the default and haven't noticed it. When I had a quick look into the issue I just mentioned, it seemed to be being caused by the wrong number being reported. I think that would cause a similar issue as what you are describing, but it could also be something entirely different.

g-erson commented 5 years ago

Alright after some testing I've observed a few things with "tabs.mode" : buffer.

If you open some buffers with quickopen, files A, B, and C, they will open in tabs 1, 2, and 3.

At this point we can use gt/gT to switch between tabs as expected.

Now use quickopen to open file B again. This means (I'm guessing a bit here...) a new buffer is opened again and joined to tab 2. Now tab 2 has two buffers attached to it.

Now things get a bit weird... gt/gT seem to switch not between tabs, but buffers. So now we switch between tabs 1, 2, 3, 2, 1, 2, 3, 2, 1... in that order. Calling ':close' on the tab with 2 buffers open for the same file causes gt/gT to cycle in the normal way again. Calling ':close' a second time causes us to no longer cycle through that tab with gt/gT, but we can still click on it, which switches which 2 tabs then get cycled through.

Calling ':bd' to close the buffer causes other odd behaviour w.r.t tab switching. As does switching buffers with ':b'.

Would it make more sense if;

?

If quickopen always opened buffers in a new tab, but never opened a new buffer of the same file twice, then tab switching could work by just typing part of a name in quickopen, as well as using gt/gT.

What do you think?

akinsho commented 5 years ago

@g-erson thanks for the debugging 👍 from what you've described it seems to me like the method we're using to open buffers might actually be opening new buffers in between tabs we have logic that is supposed to handle opening buffers if the mode if open mode is buffers and vice versa but seems like we might that logic might be jumbled up at some point possibly in NeovimEditor.ts

CrossR commented 5 years ago

At this point we can use gt/gT to switch between tabs as expected.

Actually, I wouldn't expect gT/gt to work when the mode is set to "buffers", since those binds are for moving between tabs, not between buffers. (When the oni mode is set to "buffers", Oni will show tabs, but they aren't "real" tabs, just Oni showing each buffer in its own tab).

There is also going to probably be some issues with the following bit of code, which for the tab case, does swap to the existing if it can, but for the buffer case, it does not I don't think?

                [Oni.FileOpenMode.Edit]: tabsMode ? "tab drop" : "e!",

I admit, I swapped to tabs and have just been using that, despite not initially really wanting to swap.

akinsho commented 5 years ago

I've done a bit of debugging myself and I think when the QuickOpen menu was refactored the command it was set to use somehow got muddled as when I log out the command being used by the quick open for me it turns out its using tabnew! it seems for users using tabs.mode: "buffers" there is now another setting that is being taken into account i.e. quickOpen.defaultOpenMode which is set to tabnew by default, which in a way overrides the check in the openFile function.

Seems that we ought to somehow derive the default open mode based on what the user has set for their tabs.mode so a user isn't forced to set to different seemingly separate options in order to get consistent tab behavior