LukasPietzschmann / telescope-tabs

Fly through your tabs in NeoVim ✈️
BSD 3-Clause "New" or "Revised" License
254 stars 10 forks source link

If the last visited tab gets closed, we don't know where to switch back to when `go_to_previous` is invoked #19

Closed LukasPietzschmann closed 10 months ago

LukasPietzschmann commented 1 year ago

It would be better to keep track of a stack of recently viewed tabs so that we can always jump back to a recently closed tab, even if the last got closed.

sbeyer commented 10 months ago

Couldn't it just do while ( tab of last_index does not exist ) { last_index = last_index - 1 }? I think that would also be a sensible default.

LukasPietzschmann commented 10 months ago

Just to make sure we're on the same page: you suggest decrementing the last_index until we reach a tab that still exists?

That would not work. Imagine the following situation:

  1. You start at tab 0
  2. You click on tab 3
  3. You click on tab 4
  4. You close tab 3

Your solution would now select tab 2, which is incorrect.

If one could only navigate tabs sequentially in one direction, then your solution would work.

I hope I'm not misunderstanding or missing any information here 😅

sbeyer commented 10 months ago

I thought the indices would work differently than they are actually doing. The internal indices of telescope-tabs seem to be different and much smarter than the indices of native (neo)vim. For example, if I do :tabclose 2, the former tab 3 becomes tab 2 in (neo)vim terms, but the internal index of telescope-tabs for tab 3 is kept. Which is quite cool (and a little confusing), but I haven't noticed that before.

The thinking behind my simplistic idea was that users usually don't have a mental history of last visited tabs (except for the last tab, of course), so it's not really important which tab comes up after you've closed the tab matching your last_index. My thought was that we keep some kind of locality, i.e., if the tab with the last_index is closed, it should switch to a tab nearby (in terms of vim's indices).

But now that I got that your indices are working differently than I thought, the idea of your stack makes totally sense. I first thought that you'd need way too much bookkeeping to update indices in the stack after a tab is closed. But since you're not referring to vim's indices but your own indices, it's all fine.

Whatever solution it is, I'd prefer it over the current behavior.

LukasPietzschmann commented 10 months ago

Ah, I get your idea now. Just to clarify, I'm not implementing any smart stuff. My approach involves using tab handles instead of indices. They act more like stable IDs that remain unchanged even when tabs are closed or new ones are created. I definitely should not have named the variables last_index and current_index when they aren't indices!

While I agree that most users may not mentally track a "last visited tab stack," I am hesitant about displaying a potentially incorrect tab when navigating to the last one, even if some users may not notice.

Whatever solution it is, I'd prefer it over the current behavior.

Absolutely true. I'll work on putting together a quick implementation. :)

LukasPietzschmann commented 10 months ago

@sbeyer, If you want to, you can review the implementation I just merged and share your thoughts on it :)

sbeyer commented 10 months ago

I didn't review the implementation but from my manual tests (which reflect my main use-cases) it works like a charm :-)

LukasPietzschmann commented 10 months ago

Great! Thank you very much for your feedback :)

sbeyer commented 10 months ago

Fun fact: I noticed today that go_to_previous() is also available in native vim via g<Tab>. However, the "last visited tab gets closed" case isn't supported by vim, then g<Tab> does nothing. So now there's a good reason to override g<Tab> with the go_to_previous() functionality of your plugin. :-)

LukasPietzschmann commented 10 months ago

I did not know of g<Tab>. Good to know, that telescope-tabs improves on it. I'll immediately put this into the README xD

sbeyer commented 10 months ago

BTW: I found this out by accident because I wondered which mapping I want to use for the feature. I had tt before, but that's overriding a motion, and I thought it should be some g (goto) mapping; gt/gT is already the standard mapping for tab stuff; I use [t and ]t for those, but I didn't want to override them. So I looked for unmapped key sequences... And I thought "Tab" looks like it's a good choice for switching between tabs (it's named Tab, and it has these two arrows on it). So I tried g<Tab> and I was flashed because it already did what it should without mapping the functionality. :D