feat(tab): add keybind to go to last tab visited

[sagittarius-a]

Fixes #398.

Tab key is used as default for the GoToLastTab action.

A test have been added, All other tests still pass.

[a-kenji]

Thank you! This is already looking great.

I personally would prefer if it also kept track of the last tab
it visited while going to the last tab, so that you could easily
toggle between the two last used tabs. Do you think that might
be in scope of this pr?

If that is the case I do have these small considerations about naming.

[sagittarius-a]

Regarding the naming I'll apply your suggestions.

Regarding your comment on the "two last used tabs", I'm not sure to have properly identified your request. But to be more explicit about the current behavior:

• visit tab number 1. Let's call it dev.
• visit tab number 2. Let's call it make.
• use ToggleTab action will go to dev
• use ToggleTab again will bring you back to make

With this behavior I aimed an easy to switch between 2 tabs, hopefully staying in Tab mode would require a single Tab key press to switch between them.

Is this what you described ? If not would you mind giving an illustrated example ? :)

I'll implement it later this day if everything goes as planned.

Thank you for your time

[a-kenji]

Oh yeah, thanks that is indeed already largely how I thought it would work, i guess I was in a weird state while testing it.

My example was on creating a New Tab it would also know what the previous tab was.

Currently when opening a tab one would already need to switch once manually before being able to toggle between both of them:

• start with tab 1
• open tab 2
• toggle tab -> doesn't toggle (but I would assume it would toggle already)

Thank you and no rush!

[sagittarius-a]

I updated the PR according to your comments.

I spent some time struggling with the feature when deleting tabs between other tabs. During this process I found something quite inconsistent code:

screen.go_to_tab function is declared as follows:

    pub fn go_to_tab(&mut self, mut tab_index: usize)

The issue can be illustrated with the following scenario:

1. Create 3 tabs (Tab #1, Tab #2, Tab #3 respectively)
2. Go to tab Tab #2
3. Delete it
4. Toggle previous tab (initially using it's index)

After step 2, here is the status of tabs:

Tab: index: 0 -- position: 0 -- name: Tab #1 -- active: false -- previous: false
Tab: index: 1 -- position: 1 -- name: Tab #2 -- active: true  -- previous: false
Tab: index: 2 -- position: 2 -- name: Tab #3 -- active: false -- previous: true

After step 3, here is the status of tabs:

Tab: index: 0 -- position: 0 -- name: Tab #1 -- active: true  -- previous: false
Tab: index: 2 -- position: 1 -- name: Tab #3 -- active: false -- previous: true

As shown, for the Tab #3, index and position are now different. But if I strictly following semantic used by the screen.go_to_tab function, I use the index to toggle to the previous active tab. By doing so, it fails to find the Tab #3 since the search is not performed with the index field but with position (see last line of snippet below):

    pub fn go_to_tab(&mut self, mut tab_index: usize) {
        tab_index -= 1;
        let active_tab_index = self.get_active_tab().unwrap().index;
        if let Some(t) = self.tabs.values_mut().find(|t| t.position == tab_index) {

I currently use the following code to toggle to previous active tab:

            let position = self.get_previous_tab().unwrap().position;
            self.go_to_tab(position + 1);

Maybe the semantic for the screen.go_to_tab could be updated to use position instead of index ? screen.active_tab_index and screen.previous_active_tab_index could be renamed to something closer from position than index ?

I'm not sure it fits in the current PR but I wanted to address it here since it is closely related to the changes performed.

Thanks !

[a-kenji]

Thank you for the changes, this is looking much better to me.
There is still a situation where closing a Tab will render the toggle useless,
and I think this is currently the best we are going to get while saving a single number.

I think it could be beneficial here to use a Vec as a stack and push on creation,
pop on delete and pop & push on toggle, what do you think? Maybe you will find
a more elegant way.

This is the situation that currently fails:

• open tab 1
• open tab 2
• open tab 3
• GoToTab 1
• CloseTab
• ToggleTab

Good find on that index inconsistency! If I understand you correctly
I think I agree that changing it is the best course of action for now, I think the index
is there to have a unique number associated with each tab that stays the same regardless
of reordering. But using that number for a go_to_tab action might not be the best choice.

Please do so In a separate commit and put the awesome diagram in your message,
if you put it here, or in a separate pr is up to you.

[sagittarius-a]

I finally took the time to work on this !

I will address a new PR for the variable naming issue, I find it more relevant.

Let me know if you find some improvements that can be made :)

[a-kenji]

@sagittarius-a,
Thank you for this awesome work! This looks great to me.