Repository: zellij-org/zellij
File: zellij-server/src/route.rs (Rust)

Code before:
    ServerInstruction, SessionMetaData, SessionState,
};
use zellij_utils::{
    channels::SenderWithContext,
    data::Event,
    errors::prelude::*,
    input::{
        actions::{Action, Direction, ResizeDirection, SearchDirection, SearchOption},
        command::TerminalAction,
        get_mode_info,
    },
    ipc::{ClientToServerMsg, ExitReason, IpcReceiverWithContext, ServerToClientMsg},
};

use crate::ClientId;

pub(crate) fn route_action(
    action: Action,
    session: &SessionMetaData,
    _os_input: &dyn ServerOsApi,
    to_server: &SenderWithContext<ServerInstruction>,
    client_id: ClientId,
) -> Result<bool> {
    let mut should_break = false;

    // forward the action to plugins unless it is a mousehold
    // this is a bit of a hack around the unfortunate architecture we use with plugins
    // this will change as soon as we refactor
    match action {
        Action::MouseHoldLeft(..) | Action::MouseHoldRight(..) => {},
        _ => {
            session.senders.send_to_plugin(PluginInstruction::Update(
                None,
                Some(client_id),
                Event::InputReceived,
            ))?;
        },
    }

    match action {
        Action::ToggleTab => {
            session
                .senders
                .send_to_screen(ScreenInstruction::ToggleTab(client_id))?;
        },
        Action::Write(val) => {
            session
                .senders
                .send_to_screen(ScreenInstruction::ClearScroll(client_id))?;
            session
                .senders

Review comment (bug):
Sorry for not raising this earlier... I think we should add an error context to all the `?` inside this function. I missed that this actually *isn't* the threads main function, but only called from `route_thread_main`. I'm fine with sticking to `?` in `route_thread_main` because we will call `.fatal()` on it in the next step and it's totally clear where the error comes from and what we tried to do.

I think an error context like this:

```suggestion
let err_context = || format!("failed to route action for client {client_id}: {action:?}");
```

should do, and then you replace all `?` in this function with `.with_context(err_context)?`. Is that alright for you?

Code after:
    ServerInstruction, SessionMetaData, SessionState,
};
use zellij_utils::{
    channels::SenderWithContext,
    data::Event,
    errors::prelude::*,
    input::{
        actions::{Action, Direction, ResizeDirection, SearchDirection, SearchOption},
        command::TerminalAction,
        get_mode_info,
    },
    ipc::{ClientToServerMsg, ExitReason, IpcReceiverWithContext, ServerToClientMsg},
};

use crate::ClientId;

pub(crate) fn route_action(
    action: Action,
    session: &SessionMetaData,
    _os_input: &dyn ServerOsApi,
    to_server: &SenderWithContext<ServerInstruction>,
    client_id: ClientId,
) -> Result<bool> {
    let mut should_break = false;
    let err_context = || format!("failed to route action for client {client_id}");

    // forward the action to plugins unless it is a mousehold
    // this is a bit of a hack around the unfortunate architecture we use with plugins
    // this will change as soon as we refactor
    match action {
        Action::MouseHoldLeft(..) | Action::MouseHoldRight(..) => {},
        _ => {
            session
                .senders
                .send_to_plugin(PluginInstruction::Update(
                    None,
                    Some(client_id),
                    Event::InputReceived,
                ))
                .with_context(err_context)?;
        },
    }

    match action {
        Action::ToggleTab => {
            session
                .senders
                .send_to_screen(ScreenInstruction::ToggleTab(client_id))
                .with_context(err_context)?;
        },
        Action::Write(val) => {