Removing a tab causes pane focus to shift to the next tab

[SecretPocketCat]

2. Issues with the Zellij UI / behavior / crash

Issue description

Removing a tab causes pane focus to shift to the next tab

Minimal reproduction

1. Run the following plugin using the provided template
2. press alt-f to focus the terminal pane in the Tabby 3 - focused tab (this is not needed, but it shows it still behaves as expected at this point)
3. press alt-d to close the Tabby 2 - closed tab
4. press alt-f which should focus the terminal pane in the Tabby 3 - focused tab again, but it focuses Tabby 4 instead

Plugin

use std::collections::BTreeMap;
use zellij_tile::prelude::*;

#[derive(Default)]
struct PluginState {
    focused_tab: Option<usize>,
    kb: bool,
    term_panes: Vec<(usize, u32)>,
}

register_plugin!(PluginState);

impl ZellijPlugin for PluginState {
    fn load(&mut self, _configuration: BTreeMap<String, String>) {
        request_permission(&[
            PermissionType::ReadApplicationState,
            PermissionType::ChangeApplicationState,
        ]);
        subscribe(&[EventType::TabUpdate, EventType::PaneUpdate]);
    }

    fn update(&mut self, event: Event) -> bool {
        match event {
            Event::TabUpdate(tabs) => {
                self.focused_tab = tabs
                    .iter()
                    .enumerate()
                    .find(|(_, t)| t.active)
                    .map(|(i, _)| i);
            }
            Event::PaneUpdate(PaneManifest { panes }) => {
                let id = get_plugin_ids().plugin_id;
                self.kb = panes
                    .get(&0)
                    .unwrap()
                    .iter()
                    .find(|p| p.is_plugin && p.id == id)
                    .is_some();

                self.term_panes = panes
                    .iter()
                    .flat_map(|(i, panes)| {
                        panes
                            .iter()
                            .filter(|p| !p.is_plugin)
                            .map(move |p| (*i, p.id))
                    })
                    .collect();
            }
            _ => unimplemented!("{event:?}"),
        };

        true
    }

    fn pipe(&mut self, pipe_message: PipeMessage) -> bool {
        if !self.kb {
            return true;
        }

        if pipe_message.name == "close" {
            go_to_tab(1);
            close_focused_tab();
        } else {
            focus_terminal_pane(2, false);
        }
        true
    }

    fn render(&mut self, _rows: usize, _cols: usize) {
        println!("Focused tab: {:?}", self.focused_tab);
        println!("Term pane IDs: {:?}", self.term_panes);
    }
}

Template

layout {
    tab_template name="plugin_tab" {
        pane {
            pane {}
            pane {
                plugin location="file:target/wasm32-wasi/debug/repro.wasm"
            }
            children
        }
        pane size=1 borderless=true {
            plugin location="tab-bar"
        }
    }

    plugin_tab name="Tabby 1 - skipped"
    plugin_tab name="Tabby 2 - closed"
    plugin_tab name="Tabby 3 - focused"
    plugin_tab name="Tabby 4"
}  

keybinds {
    shared {
        bind "Alt f" {
            MessagePlugin "file:target/wasm32-wasi/debug/repro.wasm" {
                name "focus"
            }
        }
        bind "Alt d" {
            MessagePlugin "file:target/wasm32-wasi/debug/repro.wasm" {
                name "close"
            }
        }
    }
}

Other relevant information

Happens with #3299 too.
Based on testing with a different plugin, the pane is focused, but it's immediately followed by the incorrect focus of the next pane.

[imsnif]

Hey, the reproduction is a bit involved so it'll take me some time to get to it - but I want to give you a quick answer so I'm going to guess a little:

1. Order is not guaranteed in plugin commands - so doing things like go_to_tab(1) and close_focused_tab() might not have the desired result. I hope to make this easier in the future by allowing plugins to initiate transactions (similar to databases), but for now - if this is the issue - I'd recommend only closing the focused tab after parsing the application state and making sure the desired tab is indeed focused.
2. When a tab is closed, all the clients focused on it are moved to their previously focused tab

I hope one of these can help explain and work around your issue?

[SecretPocketCat]

the reproduction is a bit involved

Yeah, sorry about that, I wanted to provide a complete repro. The TLDR is that focusing a specific pane after removing a tab focuses the next pane (+1 from the expected one).

1. I wasn't aware of that, it just happened to work out every time. I count on that assumption in my other plugin, so I might have to rewrite that. I don't it's super relevant in the repro though, as the tabs are named, so it'd be clear when the closing does not happen in the expected order.
2. What does client refer to in this case? The relevant step is the `focus_terminal' which can follow an arbitrary amount of time after closing the tab, though. Can pane IDs actually change, ie. can I store stale IDs in the plugin?

[imsnif]

Hey @SecretPocketCat - I tried your reproduction and for me it seems to work as expected (again, the example is a bit involved - if I understand the expected behavior correctly - I remain focused on the same pane after closing the tab).

To me this indicates this is likely some sort of race (either between events or between plugin instances, since this example has 4 of them open). Would you maybe like to try and troubleshoot this a little? Remove as many elements as possible and find a minimal reproduction? I'd recommend trying to debug print the various state events you get (from PaneInfo and TabInfo) between each command send to Zellij, to make 100% sure the state looks like you expect it to. Also, try running with just 1 plugin instance.

2. What does client refer to in this case? The relevant step is the `focus_terminal' which can follow an arbitrary amount of time after closing the tab, though.  Can pane IDs actually change, ie. can I store stale IDs in the plugin?

A client is the connected user to the session (eg. if you attach from two different terminal windows to the same session, there will be 2 different clients connected).

[SecretPocketCat]

I'm not sure I've explained the issue clearly. My usecase is storing a pane ID (e.g. for an editor pane) for each tab and then jumping to that pane by using focus_terminal_pane with that stored ID. That works a treat until a tab is closed. After closing a tab which precedes the tab with the target pane (with the stored ID) actually focuses the following tab, not the one containing the pane. Does that make sense?

My assumption was the pane IDs are stable, but that might not be the case and are shifted when the tab is closed?

I revamped the repro to be simpler and use just 1 plugin instance:

Template

Each tab tails the zellij log. I'm not sure if the dir name changes, so don't forget to update it if it's different for you. Other than that, the first tab contains the plugin.

layout {
    tab_template name="log" {
        pane {
            children
            pane name="log" {
                command "tail"
                args "/tmp/zellij-1000/zellij-log/zellij.log" "-F"
            }
        }
        pane size=1 borderless=true {
            plugin location="tab-bar"
        }
    }

    log name="Plugin" {
        pane {
            plugin location="file:target/wasm32-wasi/debug/repro.wasm"
        }
    }
    log name="Close me" focus=true
    log name="Contains focused pane"
    log name="After focus pane"
}  

keybinds {
    shared {
        bind "Alt f" {
            MessagePlugin "file:target/wasm32-wasi/debug/repro.wasm" {
                name "focus"
            }
        }
        bind "Alt d" { CloseTab; }
    }
}

Plugin

The plugin logs tab focus changes and the tab of the focused pane - the ID is fixed to illustrate the issue.
In addition to that the plugin focuses the terminal pane by using a named pipe message bound to alt f.

use std::collections::BTreeMap;
use zellij_tile::prelude::*;

const FOCUSED_TERM_ID: u32 = 2;

#[derive(Default)]
struct PluginState {
    focused_tab: Option<usize>,
}

register_plugin!(PluginState);

impl ZellijPlugin for PluginState {
    fn load(&mut self, _configuration: BTreeMap<String, String>) {
        request_permission(&[
            PermissionType::ReadApplicationState,
            PermissionType::ChangeApplicationState,
        ]);
        subscribe(&[EventType::TabUpdate, EventType::PaneUpdate]);
    }

    fn update(&mut self, event: Event) -> bool {
        match event {
            Event::TabUpdate(tabs) => {
                let prev = self.focused_tab.clone();
                self.focused_tab = tabs
                    .iter()
                    .enumerate()
                    .find(|(_, t)| t.active)
                    .map(|(i, _)| i);
                eprintln!("Tab update: [{prev:?}] => [{:?}]", self.focused_tab);
            }
            Event::PaneUpdate(PaneManifest { panes }) => {
                let pane_tab = panes
                    .iter()
                    .flat_map(|(i, panes)| panes.iter().map(move |p| (i, p)))
                    .find(|(_, p)| p.id == FOCUSED_TERM_ID && !p.is_plugin)
                    .map(|(i, _)| i);
                eprintln!("Pane update: pane is at tab [{pane_tab:?}]");
            }
            _ => unimplemented!("{event:?}"),
        };

        false
    }

    fn pipe(&mut self, pipe_message: PipeMessage) -> bool {
        if pipe_message.name == "focus" {
            focus_terminal_pane(FOCUSED_TERM_ID, false);
        }

        false
    }
}

Repro

1. Run the plugin using the provided template
2. Close the Close me tab (using the alt d or whichever binding you use normally)
3. press alt-f to focus the terminal pane (id = 2)

Expected behaviour

The pane and in turn the Contains focused pane tab is focused (index 1 at this point).
The log is (the initial might be smt. different ofc)

DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:02:29.983 [id: 0     ] Tab update: [Some(1)] => [Some(2)]
DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:02:29.985 [id: 0     ] Pane update: pane is at tab [Some(2)]
DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:02:29.986 [id: 0     ] Tab update: [Some(2)] => [Some(2)]

This works before closing tab Close me

Actual behaviour

The After focus pane tab is focused.
The log is (notice pane is at tab [Some(1)], but the focused tab is 2)

DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:05:01.057 [id: 0     ] Tab update: [Some(0)] => [Some(2)]
DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:05:01.058 [id: 0     ] Pane update: pane is at tab [Some(1)]
DEBUG  |target/wasm32-wasi/debug/| 2024-04-27 10:05:01.058 [id: 0     ] Tab update: [Some(2)] => [Some(2)]