fix(cli): respect cwd in zellij run and zellij plugin (#3116)

* fix(cli): respect cwd in zellij run and zellij plugin commands

* style(fmt): rustfmt

* fix tests
This commit is contained in:
Aram Drevekenin 2024-02-07 10:39:51 +01:00 committed by GitHub
parent 5e364940fd
commit 7e549cdbd5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 124 additions and 59 deletions

View file

@ -31,6 +31,7 @@ fn main() {
start_suspended,
})) = opts.command
{
let cwd = cwd.or_else(|| std::env::current_dir().ok());
let skip_plugin_cache = false; // N/A for this action
let command_cli_action = CliAction::NewPane {
command,
@ -56,11 +57,12 @@ fn main() {
skip_plugin_cache,
})) = opts.command
{
let cwd = std::env::current_dir().ok();
let command_cli_action = CliAction::NewPane {
command: vec![],
plugin: Some(url),
direction: None,
cwd: None,
cwd,
floating,
in_place,
name: None,

View file

@ -3841,7 +3841,11 @@ pub fn open_file_floating_plugin_command() {
}
})
.clone();
assert_snapshot!(format!("{:#?}", new_tab_event));
// we do the replace below to avoid the randomness of the temporary folder in the snapshot
// while still testing it
assert_snapshot!(
format!("{:#?}", new_tab_event).replace(&format!("{:?}", temp_folder.path()), "\"CWD\"")
);
}
#[test]
@ -3918,7 +3922,11 @@ pub fn open_file_plugin_command() {
}
})
.clone();
assert_snapshot!(format!("{:#?}", new_tab_event));
// we do the replace below to avoid the randomness of the temporary folder in the snapshot
// while still testing it
assert_snapshot!(
format!("{:#?}", new_tab_event).replace(&format!("{:?}", temp_folder.path()), "\"CWD\"")
);
}
#[test]
@ -3996,7 +4004,11 @@ pub fn open_file_with_line_plugin_command() {
}
})
.clone();
assert_snapshot!(format!("{:#?}", new_tab_event));
// we do the replace below to avoid the randomness of the temporary folder in the snapshot
// while still testing it
assert_snapshot!(
format!("{:#?}", new_tab_event).replace(&format!("{:?}", temp_folder.path()), "\"CWD\"")
);
}
#[test]
@ -4073,7 +4085,11 @@ pub fn open_file_with_line_floating_plugin_command() {
}
})
.clone();
assert_snapshot!(format!("{:#?}", new_tab_event));
// we do the replace below to avoid the randomness of the temporary folder in the snapshot
// while still testing it
assert_snapshot!(
format!("{:#?}", new_tab_event).replace(&format!("{:?}", temp_folder.path()), "\"CWD\"")
);
}
#[test]

View file

@ -1,7 +1,7 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 2820
expression: "format!(\"{:#?}\", new_tab_event)"
assertion_line: 3846
expression: "format!(\"{:#?}\",\n new_tab_event).replace(&format!(\"{:?}\", temp_folder.path()),\n \"\\\"CWD\\\"\")"
---
Some(
SpawnTerminal(
@ -9,7 +9,9 @@ Some(
OpenFile(
"/path/to/my/file.rs",
None,
None,
Some(
"CWD",
),
),
),
Some(

View file

@ -1,7 +1,7 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 2874
expression: "format!(\"{:#?}\", new_tab_event)"
assertion_line: 3925
expression: "format!(\"{:#?}\",\n new_tab_event).replace(&format!(\"{:?}\", temp_folder.path()),\n \"\\\"CWD\\\"\")"
---
Some(
SpawnTerminal(
@ -9,7 +9,9 @@ Some(
OpenFile(
"/path/to/my/file.rs",
None,
None,
Some(
"CWD",
),
),
),
Some(

View file

@ -1,7 +1,7 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 2982
expression: "format!(\"{:#?}\", new_tab_event)"
assertion_line: 4076
expression: "format!(\"{:#?}\",\n new_tab_event).replace(&format!(\"{:?}\", temp_folder.path()),\n \"\\\"CWD\\\"\")"
---
Some(
SpawnTerminal(
@ -11,7 +11,9 @@ Some(
Some(
42,
),
None,
Some(
"CWD",
),
),
),
Some(

View file

@ -1,7 +1,7 @@
---
source: zellij-server/src/plugins/./unit/plugin_tests.rs
assertion_line: 2927
expression: "format!(\"{:#?}\", new_tab_event)"
assertion_line: 3999
expression: "format!(\"{:#?}\",\n new_tab_event).replace(&format!(\"{:?}\", temp_folder.path()),\n \"\\\"CWD\\\"\")"
---
Some(
SpawnTerminal(
@ -11,7 +11,9 @@ Some(
Some(
42,
),
None,
Some(
"CWD",
),
),
),
Some(

View file

@ -438,7 +438,8 @@ fn open_file(env: &ForeignFunctionEnv, file_to_open: FileToOpen) {
let path = env.plugin_env.plugin_cwd.join(file_to_open.path);
let cwd = file_to_open
.cwd
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd));
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd))
.or_else(|| Some(env.plugin_env.plugin_cwd.clone()));
let action = Action::EditFile(
path,
file_to_open.line_number,
@ -457,7 +458,8 @@ fn open_file_floating(env: &ForeignFunctionEnv, file_to_open: FileToOpen) {
let path = env.plugin_env.plugin_cwd.join(file_to_open.path);
let cwd = file_to_open
.cwd
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd));
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd))
.or_else(|| Some(env.plugin_env.plugin_cwd.clone()));
let action = Action::EditFile(
path,
file_to_open.line_number,
@ -476,7 +478,9 @@ fn open_file_in_place(env: &ForeignFunctionEnv, file_to_open: FileToOpen) {
let path = env.plugin_env.plugin_cwd.join(file_to_open.path);
let cwd = file_to_open
.cwd
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd));
.map(|cwd| env.plugin_env.plugin_cwd.join(cwd))
.or_else(|| Some(env.plugin_env.plugin_cwd.clone()));
let action = Action::EditFile(
path,
file_to_open.line_number,

View file

@ -88,7 +88,8 @@ pub enum PtyInstruction {
Option<PaneId>, // pane id to replace if this is to be opened "in-place"
ClientId,
Size,
bool, // skip cache
bool, // skip cache
Option<PathBuf>, // if Some, will not fill cwd but just forward the message
),
Exit,
}
@ -655,6 +656,7 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box<Layout>) -> Result<()> {
client_id,
size,
skip_cache,
cwd,
) => {
pty.fill_plugin_cwd(
should_float,
@ -666,6 +668,7 @@ pub(crate) fn pty_thread_main(mut pty: Pty, layout: Box<Layout>) -> Result<()> {
client_id,
size,
skip_cache,
cwd,
)?;
},
PtyInstruction::Exit => break,
@ -1332,20 +1335,22 @@ impl Pty {
client_id: ClientId,
size: Size,
skip_cache: bool,
cwd: Option<PathBuf>,
) -> Result<()> {
let cwd = self
.active_panes
.get(&client_id)
.and_then(|pane| match pane {
PaneId::Plugin(..) => None,
PaneId::Terminal(id) => self.id_to_child_pid.get(id),
})
.and_then(|&id| {
self.bus
.os_input
.as_ref()
.and_then(|input| input.get_cwd(Pid::from_raw(id)))
});
let cwd = cwd.or_else(|| {
self.active_panes
.get(&client_id)
.and_then(|pane| match pane {
PaneId::Plugin(..) => None,
PaneId::Terminal(id) => self.id_to_child_pid.get(id),
})
.and_then(|&id| {
self.bus
.os_input
.as_ref()
.and_then(|input| input.get_cwd(Pid::from_raw(id)))
})
});
self.bus.senders.send_to_plugin(PluginInstruction::Load(
should_float,

View file

@ -659,17 +659,17 @@ pub(crate) fn route_action(
.send_to_screen(ScreenInstruction::QueryTabNames(client_id))
.with_context(err_context)?;
},
Action::NewTiledPluginPane(run_plugin, name, skip_cache) => {
Action::NewTiledPluginPane(run_plugin, name, skip_cache, cwd) => {
senders
.send_to_screen(ScreenInstruction::NewTiledPluginPane(
run_plugin, name, skip_cache, client_id,
run_plugin, name, skip_cache, cwd, client_id,
))
.with_context(err_context)?;
},
Action::NewFloatingPluginPane(run_plugin, name, skip_cache) => {
Action::NewFloatingPluginPane(run_plugin, name, skip_cache, cwd) => {
senders
.send_to_screen(ScreenInstruction::NewFloatingPluginPane(
run_plugin, name, skip_cache, client_id,
run_plugin, name, skip_cache, cwd, client_id,
))
.with_context(err_context)?;
},
@ -708,7 +708,7 @@ pub(crate) fn route_action(
))
.with_context(err_context)?;
},
Action::LaunchPlugin(run_plugin, should_float, should_open_in_place, skip_cache) => {
Action::LaunchPlugin(run_plugin, should_float, should_open_in_place, skip_cache, cwd) => {
senders
.send_to_screen(ScreenInstruction::LaunchPlugin(
run_plugin,
@ -716,6 +716,7 @@ pub(crate) fn route_action(
should_open_in_place,
pane_id,
skip_cache,
cwd,
client_id,
))
.with_context(err_context)?;

View file

@ -272,11 +272,11 @@ pub enum ScreenInstruction {
PreviousSwapLayout(ClientId),
NextSwapLayout(ClientId),
QueryTabNames(ClientId),
NewTiledPluginPane(RunPlugin, Option<String>, bool, ClientId), // Option<String> is
// optional pane title, bool is skip cache
NewFloatingPluginPane(RunPlugin, Option<String>, bool, ClientId), // Option<String> is an
NewTiledPluginPane(RunPlugin, Option<String>, bool, Option<PathBuf>, ClientId), // Option<String> is
// optional pane title, bool is skip cache, Option<PathBuf> is an optional cwd
NewFloatingPluginPane(RunPlugin, Option<String>, bool, Option<PathBuf>, ClientId), // Option<String> is an
// optional pane title, bool
// is skip cache
// is skip cache, Option<PathBuf> is an optional cwd
NewInPlacePluginPane(RunPlugin, Option<String>, PaneId, bool, ClientId), // Option<String> is an
// optional pane title, bool is skip cache
StartOrReloadPluginPane(RunPlugin, Option<String>),
@ -296,9 +296,17 @@ pub enum ScreenInstruction {
ProgressPluginLoadingOffset(u32), // u32 - plugin id
RequestStateUpdateForPlugins,
LaunchOrFocusPlugin(RunPlugin, bool, bool, bool, Option<PaneId>, bool, ClientId), // bools are: should_float, move_to_focused_tab, should_open_in_place, Option<PaneId> is the pane id to replace, bool following it is skip_cache
LaunchPlugin(RunPlugin, bool, bool, Option<PaneId>, bool, ClientId), // bools are: should_float, should_open_in_place Option<PaneId> is the pane id to replace, bool after is skip_cache
SuppressPane(PaneId, ClientId), // bool is should_float
FocusPaneWithId(PaneId, bool, ClientId), // bool is should_float
LaunchPlugin(
RunPlugin,
bool,
bool,
Option<PaneId>,
bool,
Option<PathBuf>,
ClientId,
), // bools are: should_float, should_open_in_place Option<PaneId> is the pane id to replace, Option<PathBuf> is an optional cwd, bool after is skip_cache
SuppressPane(PaneId, ClientId), // bool is should_float
FocusPaneWithId(PaneId, bool, ClientId), // bool is should_float
RenamePane(PaneId, Vec<u8>),
RenameTab(usize, Vec<u8>),
RequestPluginPermissions(
@ -3212,6 +3220,7 @@ pub(crate) fn screen_thread_main(
run_plugin,
pane_title,
skip_cache,
cwd,
client_id,
) => {
let tab_index = screen.active_tab_indices.values().next().unwrap_or(&1);
@ -3231,12 +3240,14 @@ pub(crate) fn screen_thread_main(
client_id,
size,
skip_cache,
cwd,
))?;
},
ScreenInstruction::NewFloatingPluginPane(
run_plugin,
pane_title,
skip_cache,
cwd,
client_id,
) => match screen.active_tab_indices.values().next() {
Some(tab_index) => {
@ -3256,6 +3267,7 @@ pub(crate) fn screen_thread_main(
client_id,
size,
skip_cache,
cwd,
))?;
},
None => {
@ -3288,6 +3300,7 @@ pub(crate) fn screen_thread_main(
client_id,
size,
skip_cache,
None,
))?;
},
None => {
@ -3358,7 +3371,7 @@ pub(crate) fn screen_thread_main(
log::error!("Must have pane id to replace or connected client_id if replacing a pane");
}
} else if let Some(client_id) = client_id {
active_tab!(screen, client_id, |active_tab: &mut Tab| {
active_tab_and_connected_client_id!(screen, client_id, |active_tab: &mut Tab, _client_id: ClientId| {
active_tab.new_pane(
PaneId::Plugin(plugin_id),
Some(pane_title),
@ -3448,6 +3461,7 @@ pub(crate) fn screen_thread_main(
client_id,
size,
skip_cache,
None,
))?;
},
None => {
@ -3492,6 +3506,7 @@ pub(crate) fn screen_thread_main(
client_id,
Size::default(),
skip_cache,
None,
))?;
}
},
@ -3507,6 +3522,7 @@ pub(crate) fn screen_thread_main(
should_open_in_place,
pane_id_to_replace,
skip_cache,
cwd,
client_id,
) => match pane_id_to_replace {
Some(pane_id_to_replace) => match screen.active_tab_indices.values().next() {
@ -3525,6 +3541,7 @@ pub(crate) fn screen_thread_main(
client_id,
size,
skip_cache,
cwd,
))?;
},
None => {
@ -3560,6 +3577,7 @@ pub(crate) fn screen_thread_main(
client_id,
Size::default(),
skip_cache,
cwd,
))?;
},
None => {

View file

@ -1,6 +1,6 @@
---
source: zellij-server/src/./unit/screen_tests.rs
assertion_line: 2620
assertion_line: 2627
expression: "format!(\"{:#?}\", pty_fill_plugin_cwd_instruction)"
---
Some(
@ -27,5 +27,6 @@ Some(
cols: 0,
},
false,
None,
),
)

View file

@ -213,8 +213,8 @@ pub enum Action {
MiddleClick(Position),
LaunchOrFocusPlugin(RunPlugin, bool, bool, bool, bool), // bools => should float,
// move_to_focused_tab, should_open_in_place, skip_cache
LaunchPlugin(RunPlugin, bool, bool, bool), // bools => should float,
// should_open_in_place, skip_cache
LaunchPlugin(RunPlugin, bool, bool, bool, Option<PathBuf>), // bools => should float,
// should_open_in_place, skip_cache, Option<PathBuf> is cwd
LeftMouseRelease(Position),
RightMouseRelease(Position),
MiddleMouseRelease(Position),
@ -240,10 +240,10 @@ pub enum Action {
/// Query all tab names
QueryTabNames,
/// Open a new tiled (embedded, non-floating) plugin pane
NewTiledPluginPane(RunPlugin, Option<String>, bool), // String is an optional name, bool is
// skip_cache
NewFloatingPluginPane(RunPlugin, Option<String>, bool), // String is an optional name, bool is
// skip_cache
NewTiledPluginPane(RunPlugin, Option<String>, bool, Option<PathBuf>), // String is an optional name, bool is
// skip_cache, Option<PathBuf> is cwd
NewFloatingPluginPane(RunPlugin, Option<String>, bool, Option<PathBuf>), // String is an optional name, bool is
// skip_cache, Option<PathBuf> is cwd
NewInPlacePluginPane(RunPlugin, Option<String>, bool), // String is an optional name, bool is
// skip_cache
StartOrReloadPlugin(RunPlugin),
@ -337,7 +337,7 @@ impl Action {
.or_else(|| Some(current_dir));
let user_configuration = configuration.unwrap_or_default();
if let Some(plugin) = plugin {
let location = RunPluginLocation::parse(&plugin, cwd)
let location = RunPluginLocation::parse(&plugin, cwd.clone())
.map_err(|e| format!("Failed to parse plugin loction {plugin}: {}", e))?;
let plugin = RunPlugin {
_allow_exec_host_cmd: false,
@ -349,6 +349,7 @@ impl Action {
plugin,
name,
skip_plugin_cache,
cwd,
)])
} else if in_place {
Ok(vec![Action::NewInPlacePluginPane(
@ -369,6 +370,7 @@ impl Action {
plugin,
name,
skip_plugin_cache,
cwd,
)])
}
} else if !command.is_empty() {
@ -583,8 +585,9 @@ impl Action {
skip_plugin_cache,
} => {
let current_dir = get_current_dir();
let run_plugin_location = RunPluginLocation::parse(url.as_str(), Some(current_dir))
.map_err(|e| format!("Failed to parse plugin location: {}", e))?;
let run_plugin_location =
RunPluginLocation::parse(url.as_str(), Some(current_dir.clone()))
.map_err(|e| format!("Failed to parse plugin location: {}", e))?;
let run_plugin = RunPlugin {
location: run_plugin_location,
_allow_exec_host_cmd: false,
@ -595,6 +598,7 @@ impl Action {
floating,
in_place,
skip_plugin_cache,
Some(current_dir),
)])
},
CliAction::RenameSession { name } => Ok(vec![Action::RenameSession(name)]),

View file

@ -992,6 +992,8 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
should_float,
should_open_in_place,
skip_plugin_cache,
None, // we explicitly do not send the current dir here so that it will be
// filled from the active pane == better UX
))
},
"PreviousSwapLayout" => Ok(Action::PreviousSwapLayout),

View file

@ -439,6 +439,7 @@ impl TryFrom<ProtobufAction> for Action {
should_float,
should_open_in_place,
skip_plugin_cache,
None,
))
},
_ => Err("Wrong payload for Action::LaunchOrFocusPlugin"),
@ -548,6 +549,7 @@ impl TryFrom<ProtobufAction> for Action {
run_plugin,
pane_name,
skip_plugin_cache,
None,
))
},
_ => Err("Wrong payload for Action::NewTiledPluginPane"),
@ -570,6 +572,7 @@ impl TryFrom<ProtobufAction> for Action {
run_plugin,
pane_name,
skip_plugin_cache,
None,
))
},
_ => Err("Wrong payload for Action::MiddleClick"),
@ -1043,6 +1046,7 @@ impl TryFrom<Action> for ProtobufAction {
should_float,
should_open_in_place,
skip_plugin_cache,
cwd,
) => {
let url: Url = Url::from(&run_plugin.location);
Ok(ProtobufAction {
@ -1137,7 +1141,7 @@ impl TryFrom<Action> for ProtobufAction {
name: ProtobufActionName::QueryTabNames as i32,
optional_payload: None,
}),
Action::NewTiledPluginPane(run_plugin, pane_name, skip_plugin_cache) => {
Action::NewTiledPluginPane(run_plugin, pane_name, skip_plugin_cache, _cwd) => {
let plugin_url: Url = Url::from(&run_plugin.location);
Ok(ProtobufAction {
name: ProtobufActionName::NewTiledPluginPane as i32,
@ -1150,7 +1154,7 @@ impl TryFrom<Action> for ProtobufAction {
)),
})
},
Action::NewFloatingPluginPane(run_plugin, pane_name, skip_plugin_cache) => {
Action::NewFloatingPluginPane(run_plugin, pane_name, skip_plugin_cache, _cwd) => {
let plugin_url: Url = Url::from(&run_plugin.location);
Ok(ProtobufAction {
name: ProtobufActionName::NewFloatingPluginPane as i32,