zellij-org/zellij
1
1
Fork 0
mirror of https://github.com/zellij-org/zellij.git synced 2024-11-25 23:33:08 +03:00
Code Issues Projects Releases Wiki Activity

feat(cli): make --layout idempotent(-ish) (#3650)

Browse Source 
* feat(cli): if inside a session, apply --layout to the session

* fix(screen): some focusing races when switching tab focus

* style(fmt): rustfmt
This commit is contained in:
Aram Drevekenin 2024-10-08 16:57:54 +02:00 committed by GitHub
parent 4ac7d08658
commit 203fbf7a49
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 161 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
default-plugins/status-bar/src/one_line_ui.rs
View File

@ -1193,7 +1193,7 @@ fn get_keys_and_hints(mi: &ModeInfo) -> Vec<(String, String, Vec<KeyWithModifier
};
vec![
(s("New"), s("New"), single_action_key(&km, &[A::NewTab(None, vec![], None, None, None), TO_NORMAL])),
(s("New"), s("New"), single_action_key(&km, &[A::NewTab(None, vec![], None, None, None, true), TO_NORMAL])),
(s("Change focus"), s("Move"), focus_keys),
(s("Close"), s("Close"), single_action_key(&km, &[A::CloseTab, TO_NORMAL])),
(s("Rename"), s("Rename"),
@ -1275,7 +1275,7 @@ fn get_keys_and_hints(mi: &ModeInfo) -> Vec<(String, String, Vec<KeyWithModifier
(s("Split down"), s("Down"), action_key(&km, &[A::NewPane(Some(Dir::Down), None, false), TO_NORMAL])),
(s("Split right"), s("Right"), action_key(&km, &[A::NewPane(Some(Dir::Right), None, false), TO_NORMAL])),
(s("Fullscreen"), s("Fullscreen"), action_key(&km, &[A::ToggleFocusFullscreen, TO_NORMAL])),
(s("New tab"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None), TO_NORMAL])),
(s("New tab"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None, true), TO_NORMAL])),
(s("Rename tab"), s("Rename"),
action_key(&km, &[A::SwitchToMode(IM::RenameTab), A::TabNameInput(vec![0])])),
(s("Previous Tab"), s("Previous"), action_key(&km, &[A::GoToPreviousTab, TO_NORMAL])),

4
default-plugins/status-bar/src/second_line.rs
View File

@ -179,7 +179,7 @@ fn get_keys_and_hints(mi: &ModeInfo) -> Vec<(String, String, Vec<KeyWithModifier
};
vec![
(s("New"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None), TO_NORMAL])),
(s("New"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None, true), TO_NORMAL])),
(s("Change focus"), s("Move"), focus_keys),
(s("Close"), s("Close"), action_key(&km, &[A::CloseTab, TO_NORMAL])),
(s("Rename"), s("Rename"),
@ -259,7 +259,7 @@ fn get_keys_and_hints(mi: &ModeInfo) -> Vec<(String, String, Vec<KeyWithModifier
(s("Split down"), s("Down"), action_key(&km, &[A::NewPane(Some(Dir::Down), None, false), TO_NORMAL])),
(s("Split right"), s("Right"), action_key(&km, &[A::NewPane(Some(Dir::Right), None, false), TO_NORMAL])),
(s("Fullscreen"), s("Fullscreen"), action_key(&km, &[A::ToggleFocusFullscreen, TO_NORMAL])),
(s("New tab"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None), TO_NORMAL])),
(s("New tab"), s("New"), action_key(&km, &[A::NewTab(None, vec![], None, None, None, true), TO_NORMAL])),
(s("Rename tab"), s("Rename"),
action_key(&km, &[A::SwitchToMode(IM::RenameTab), A::TabNameInput(vec![0])])),
(s("Previous Tab"), s("Previous"), action_key(&km, &[A::GoToPreviousTab, TO_NORMAL])),

26
src/main.rs
View File

@ -6,8 +6,10 @@ mod tests;
use zellij_utils::{
clap::Parser,
cli::{CliAction, CliArgs, Command, Sessions},
envs,
input::config::Config,
logging::*,
setup::Setup,
};
fn main() {
@ -190,6 +192,30 @@ fn main() {
commands::delete_session(target_session, force);
} else if let Some(path) = opts.server {
commands::start_server(path, opts.debug);
} else if let Some(layout) = &opts.layout {
if let Some(session_name) = opts
.session
.as_ref()
.cloned()
.or_else(|| envs::get_session_name().ok())
{
let config = Config::try_from(&opts).ok();
let options = Setup::from_cli_args(&opts).ok().map(|r| r.2);
let new_layout_cli_action = CliAction::NewTab {
layout: Some(layout.clone()),
layout_dir: options.as_ref().and_then(|o| o.layout_dir.clone()),
name: None,
cwd: options.as_ref().and_then(|o| o.default_cwd.clone()),
};
commands::send_action_to_session(new_layout_cli_action, Some(session_name), config);
} else {
commands::start_client(opts);
}
} else if let Some(layout_for_new_session) = &opts.new_session_with_layout {
let mut opts = opts.clone();
opts.new_session_with_layout = None;
opts.layout = Some(layout_for_new_session.clone());
commands::start_client(opts);
} else {
commands::start_client(opts);
}

4
src/tests/e2e/remote_runner.rs
View File

@ -111,7 +111,7 @@ fn start_zellij_mirrored_session_with_layout(channel: &mut ssh2::Channel, layout
channel
.write_all(
format!(
"{} {} --session {} --data-dir {} --layout {} options --mirror-session true --serialization-interval 1\n",
"{} {} --session {} --data-dir {} --new-session-with-layout {} options --mirror-session true --serialization-interval 1\n",
SET_ENV_VARIABLES,
ZELLIJ_EXECUTABLE_LOCATION,
SESSION_NAME,
@ -133,7 +133,7 @@ fn start_zellij_mirrored_session_with_layout_and_viewport_serialization(
channel
.write_all(
format!(
"{} {} --session {} --data-dir {} --layout {} options --mirror-session true --serialize-pane-viewport true --serialization-interval 1\n",
"{} {} --session {} --data-dir {} --new-session-with-layout {} options --mirror-session true --serialize-pane-viewport true --serialization-interval 1\n",
SET_ENV_VARIABLES,
ZELLIJ_EXECUTABLE_LOCATION,
SESSION_NAME,

29
zellij-server/src/lib.rs
View File

@ -580,7 +580,11 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
});
let cwd = runtime_config_options.default_cwd;
let spawn_tabs = |tab_layout, floating_panes_layout, tab_name, swap_layouts| {
let spawn_tabs = |tab_layout,
floating_panes_layout,
tab_name,
swap_layouts,
should_focus_tab| {
session_data
.read()
.unwrap()
@ -594,13 +598,18 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
floating_panes_layout,
tab_name,
swap_layouts,
should_focus_tab,
client_id,
))
.unwrap()
};
if layout.has_tabs() {
for (tab_name, tab_layout, floating_panes_layout) in layout.tabs() {
let focused_tab_index = layout.focused_tab_index().unwrap_or(0);
for (tab_index, (tab_name, tab_layout, floating_panes_layout)) in
layout.tabs().into_iter().enumerate()
{
let should_focus_tab = tab_index == focused_tab_index;
spawn_tabs(
Some(tab_layout.clone()),
floating_panes_layout.clone(),
@ -609,22 +618,9 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
layout.swap_tiled_layouts.clone(),
layout.swap_floating_layouts.clone(),
),
should_focus_tab,
);
}
if let Some(focused_tab_index) = layout.focused_tab_index() {
session_data
.read()
.unwrap()
.as_ref()
.unwrap()
.senders
.send_to_screen(ScreenInstruction::GoToTab(
(focused_tab_index + 1) as u32,
Some(client_id),
))
.unwrap();
}
} else {
let mut floating_panes =
layout.template.map(|t| t.1).clone().unwrap_or_default();
@ -642,6 +638,7 @@ pub fn start_server(mut os_input: Box<dyn ServerOsApi>, socket_path: PathBuf) {
layout.swap_tiled_layouts.clone(),
layout.swap_floating_layouts.clone(),
),
true,
);
}
session_data

3
zellij-server/src/plugins/unit/snapshots/zellij_server__plugins__plugin_tests__new_tab_plugin_command.snap
View File

@ -1,6 +1,6 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 573
assertion_line: 1373
expression: "format!(\"{:#?}\", new_tab_event)"
---
Some(
@ -14,6 +14,7 @@ Some(
[],
[],
),
true,
1,
),
)

3
zellij-server/src/plugins/unit/snapshots/zellij_server__plugins__plugin_tests__new_tabs_with_layout_plugin_command-2.snap
View File

@ -1,6 +1,6 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 1002
assertion_line: 1301
expression: "format!(\"{:#?}\", second_new_tab_event)"
---
Some(
@ -64,6 +64,7 @@ Some(
[],
[],
),
false,
1,
),
)

3
zellij-server/src/plugins/unit/snapshots/zellij_server__plugins__plugin_tests__new_tabs_with_layout_plugin_command.snap
View File

@ -1,6 +1,6 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 1001
assertion_line: 1300
expression: "format!(\"{:#?}\", first_new_tab_event)"
---
Some(
@ -64,6 +64,7 @@ Some(
[],
[],
),
true,
1,
),
)

10
zellij-server/src/plugins/zellij_exports.rs
View File

@ -1009,10 +1009,15 @@ fn apply_layout(env: &PluginEnv, layout: Layout) {
swap_tiled_layouts,
swap_floating_layouts,
None,
true,
);
tabs_to_open.push(action);
} else {
for (tab_name, tiled_pane_layout, floating_pane_layout) in layout.tabs() {
let focused_tab_index = layout.focused_tab_index().unwrap_or(0);
for (tab_index, (tab_name, tiled_pane_layout, floating_pane_layout)) in
layout.tabs().into_iter().enumerate()
{
let should_focus_tab = tab_index == focused_tab_index;
let swap_tiled_layouts = Some(layout.swap_tiled_layouts.clone());
let swap_floating_layouts = Some(layout.swap_floating_layouts.clone());
let action = Action::NewTab(
@ -1021,6 +1026,7 @@ fn apply_layout(env: &PluginEnv, layout: Layout) {
swap_tiled_layouts,
swap_floating_layouts,
tab_name,
should_focus_tab,
);
tabs_to_open.push(action);
}
@ -1032,7 +1038,7 @@ fn apply_layout(env: &PluginEnv, layout: Layout) {
}
fn new_tab(env: &PluginEnv) {
let action = Action::NewTab(None, vec![], None, None, None);
let action = Action::NewTab(None, vec![], None, None, None, true);
let error_msg = || format!("Failed to open new tab");
apply_action!(action, error_msg, env);
}

2
zellij-server/src/route.rs
View File

@ -499,6 +499,7 @@ pub(crate) fn route_action(
swap_tiled_layouts,
swap_floating_layouts,
tab_name,
should_change_focus_to_new_tab,
) => {
let shell = default_shell.clone();
let swap_tiled_layouts =
@ -513,6 +514,7 @@ pub(crate) fn route_action(
floating_panes_layout,
tab_name,
(swap_tiled_layouts, swap_floating_layouts),
should_change_focus_to_new_tab,
client_id,
))
.with_context(err_context)?;

28
zellij-server/src/screen.rs
View File

@ -218,6 +218,7 @@ pub enum ScreenInstruction {
Vec<FloatingPaneLayout>,
Option<String>,
(Vec<SwapTiledLayout>, Vec<SwapFloatingLayout>), // swap layouts
bool, // should_change_focus_to_new_tab
ClientId,
),
ApplyLayout(
@ -3420,12 +3421,22 @@ pub(crate) fn screen_thread_main(
floating_panes_layout,
tab_name,
swap_layouts,
should_change_focus_to_new_tab,
client_id,
) => {
let tab_index = screen.get_new_tab_index();
let should_change_focus_to_new_tab = true;
pending_tab_ids.insert(tab_index);
screen.new_tab(tab_index, swap_layouts, tab_name.clone(), Some(client_id))?;
let client_id_for_new_tab = if should_change_focus_to_new_tab {
Some(client_id)
} else {
None
};
screen.new_tab(
tab_index,
swap_layouts,
tab_name.clone(),
client_id_for_new_tab,
)?;
screen
.bus
.senders
@ -3464,6 +3475,19 @@ pub(crate) fn screen_thread_main(
for (tab_index, client_id) in pending_tab_switches.drain() {
screen.go_to_tab(tab_index as usize, client_id)?;
}
if should_change_focus_to_new_tab {
screen.go_to_tab(tab_index as usize + 1, client_id)?;
}
} else if should_change_focus_to_new_tab {
let client_id_to_switch = if screen.active_tab_indices.contains_key(&client_id)
{
Some(client_id)
} else {
screen.active_tab_indices.keys().next().copied()
};
if let Some(client_id_to_switch) = client_id_to_switch {
pending_tab_switches.insert((tab_index as usize, client_id_to_switch));
}
}
for plugin_ids in new_plugin_ids.values() {

6
zellij-server/src/unit/screen_tests.rs
View File

@ -371,6 +371,7 @@ impl MockScreen {
let default_shell = None;
let tab_name = None;
let tab_index = self.last_opened_tab_index.map(|l| l + 1).unwrap_or(0);
let should_change_focus_to_new_tab = true;
let _ = self.to_screen.send(ScreenInstruction::NewTab(
None,
default_shell,
@ -378,6 +379,7 @@ impl MockScreen {
initial_floating_panes_layout.clone(),
tab_name,
(vec![], vec![]), // swap layouts
should_change_focus_to_new_tab,
self.main_client_id,
));
let _ = self.to_screen.send(ScreenInstruction::ApplyLayout(
@ -456,6 +458,7 @@ impl MockScreen {
let default_shell = None;
let tab_name = None;
let tab_index = self.last_opened_tab_index.map(|l| l + 1).unwrap_or(0);
let should_change_focus_to_new_tab = true;
let _ = self.to_screen.send(ScreenInstruction::NewTab(
None,
default_shell,
@ -463,6 +466,7 @@ impl MockScreen {
initial_floating_panes_layout.clone(),
tab_name,
(vec![], vec![]), // swap layouts
should_change_focus_to_new_tab,
self.main_client_id,
));
let _ = self.to_screen.send(ScreenInstruction::ApplyLayout(
@ -488,6 +492,7 @@ impl MockScreen {
for i in 0..pane_count {
pane_ids.push((i as u32, None));
}
let should_change_focus_to_new_tab = true;
let _ = self.to_screen.send(ScreenInstruction::NewTab(
None,
default_shell,
@ -495,6 +500,7 @@ impl MockScreen {
vec![], // floating_panes_layout
tab_name,
(vec![], vec![]), // swap layouts
should_change_focus_to_new_tab,
self.main_client_id,
));
let _ = self.to_screen.send(ScreenInstruction::ApplyLayout(

7
zellij-utils/src/cli.rs
View File

@ -53,9 +53,16 @@ pub struct CliArgs {
pub session: Option<String>,
/// Name of a predefined layout inside the layout directory or the path to a layout file
/// if inside a session (or using the --session flag) will be added to the session as a new tab
/// or tabs, otherwise will start a new session
#[clap(short, long, value_parser, overrides_with = "layout")]
pub layout: Option<PathBuf>,
/// Name of a predefined layout inside the layout directory or the path to a layout file
/// Will always start a new session, even if inside an existing session
#[clap(short, long, value_parser, overrides_with = "new_session_with_layout")]
pub new_session_with_layout: Option<PathBuf>,
/// Change where zellij looks for the configuration file
#[clap(short, long, overrides_with = "config", env = ZELLIJ_CONFIG_FILE_ENV, value_parser)]
pub config: Option<PathBuf>,

46
zellij-utils/src/input/actions.rs
View File

@ -195,6 +195,7 @@ pub enum Action {
Option<Vec<SwapTiledLayout>>,
Option<Vec<SwapFloatingLayout>>,
Option<String>,
bool, // should_change_focus_to_new_tab
), // the String is the tab name
/// Do nothing.
NoOp,
@ -583,35 +584,58 @@ impl Action {
stringified_error
})?;
let mut tabs = layout.tabs();
if tabs.len() > 1 {
return Err(format!("Tab layout cannot itself have tabs"));
} else if !tabs.is_empty() {
if !tabs.is_empty() {
let swap_tiled_layouts = Some(layout.swap_tiled_layouts.clone());
let swap_floating_layouts = Some(layout.swap_floating_layouts.clone());
let (tab_name, layout, floating_panes_layout) =
tabs.drain(..).next().unwrap();
let name = tab_name.or(name);
Ok(vec![Action::NewTab(
let mut new_tab_actions = vec![];
let mut has_focused_tab = tabs
.iter()
.any(|(_, layout, _)| layout.focus.unwrap_or(false));
for (tab_name, layout, floating_panes_layout) in tabs.drain(..) {
let name = tab_name.or_else(|| name.clone());
let should_change_focus_to_new_tab =
layout.focus.unwrap_or_else(|| {
if !has_focused_tab {
has_focused_tab = true;
true
} else {
false
}
});
new_tab_actions.push(Action::NewTab(
Some(layout),
floating_panes_layout,
swap_tiled_layouts,
swap_floating_layouts,
swap_tiled_layouts.clone(),
swap_floating_layouts.clone(),
name,
)])
should_change_focus_to_new_tab,
));
}
Ok(new_tab_actions)
} else {
let swap_tiled_layouts = Some(layout.swap_tiled_layouts.clone());
let swap_floating_layouts = Some(layout.swap_floating_layouts.clone());
let (layout, floating_panes_layout) = layout.new_tab();
let should_change_focus_to_new_tab = true;
Ok(vec![Action::NewTab(
Some(layout),
floating_panes_layout,
swap_tiled_layouts,
swap_floating_layouts,
name,
should_change_focus_to_new_tab,
)])
}
} else {
Ok(vec![Action::NewTab(None, vec![], None, None, name)])
let should_change_focus_to_new_tab = true;
Ok(vec![Action::NewTab(
None,
vec![],
None,
None,
name,
should_change_focus_to_new_tab,
)])
}
},
CliAction::PreviousSwapLayout => Ok(vec![Action::PreviousSwapLayout]),

16
zellij-utils/src/kdl/mod.rs
View File

@ -691,12 +691,20 @@ impl Action {
Some(node)
},
Action::UndoRenamePane => Some(KdlNode::new("UndoRenamePane")),
Action::NewTab(_, _, _, _, name) => {
Action::NewTab(_, _, _, _, name, should_change_focus_to_new_tab) => {
log::warn!("Converting new tab action without arguments, original action saved to .bak.kdl file");
let mut node = KdlNode::new("NewTab");
if let Some(name) = name {
let mut children = KdlDocument::new();
let mut name_node = KdlNode::new("name");
if !should_change_focus_to_new_tab {
let mut should_change_focus_to_new_tab_node =
KdlNode::new("should_change_focus_to_new_tab");
should_change_focus_to_new_tab_node.push(KdlValue::Bool(false));
children
.nodes_mut()
.push(should_change_focus_to_new_tab_node);
}
name_node.push(name.clone());
children.nodes_mut().push(name_node);
node.set_children(children);
@ -1379,7 +1387,7 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
"NewTab" => {
let command_metadata = action_children.iter().next();
if command_metadata.is_none() {
return Ok(Action::NewTab(None, vec![], None, None, None));
return Ok(Action::NewTab(None, vec![], None, None, None, true));
}
let current_dir = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
@ -1438,6 +1446,7 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
} else if !tabs.is_empty() {
let (tab_name, layout, floating_panes_layout) = tabs.drain(..).next().unwrap();
let name = tab_name.or(name);
let should_change_focus_to_new_tab = layout.focus.unwrap_or(true);
Ok(Action::NewTab(
Some(layout),
@ -1445,9 +1454,11 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
swap_tiled_layouts,
swap_floating_layouts,
name,
should_change_focus_to_new_tab,
))
} else {
let (layout, floating_panes_layout) = layout.new_tab();
let should_change_focus_to_new_tab = layout.focus.unwrap_or(true);
Ok(Action::NewTab(
Some(layout),
@ -1455,6 +1466,7 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
swap_tiled_layouts,
swap_floating_layouts,
name,
should_change_focus_to_new_tab,
))
}
},

2
zellij-utils/src/plugin_api/action.rs
View File

@ -313,7 +313,7 @@ impl TryFrom<ProtobufAction> for Action {
Some(_) => Err("NewTab should not have a payload"),
None => {
// we do not serialize the layouts of this action
Ok(Action::NewTab(None, vec![], None, None, None))
Ok(Action::NewTab(None, vec![], None, None, None, true))
},
}
},

2
zellij-utils/src/snapshots/zellij_utils__setup__setup_test__default_config_with_no_cli_arguments.snap
View File

@ -1764,6 +1764,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,
@ -5244,6 +5245,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,

2
zellij-utils/src/snapshots/zellij_utils__setup__setup_test__layout_env_vars_override_config_env_vars.snap
View File

@ -1764,6 +1764,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,
@ -5244,6 +5245,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,

2
zellij-utils/src/snapshots/zellij_utils__setup__setup_test__layout_themes_override_config_themes.snap
View File

@ -1764,6 +1764,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,
@ -5244,6 +5245,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,

2
zellij-utils/src/snapshots/zellij_utils__setup__setup_test__layout_ui_config_overrides_config_ui_config.snap
View File

@ -1764,6 +1764,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,
@ -5244,6 +5245,7 @@ Config {
None,
None,
None,
true,
),
SwitchToMode(
Normal,