Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nushell fixes #360

Merged
merged 6 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions crates/rattler_shell/src/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ENV_START_SEPERATOR: &str = "<=== RATTLER ENV START ===>";

/// Type of modification done to the `PATH` variable
#[derive(Default, Clone)]
pub enum PathModificationBehaviour {
pub enum PathModificationBehavior {
/// Replaces the complete path variable with specified paths.
#[default]
Replace,
Expand All @@ -39,7 +39,7 @@ pub struct ActivationVariables {
pub path: Option<Vec<PathBuf>>,

/// The type of behaviour of what should happen with the defined paths.
pub path_modification_behaviour: PathModificationBehaviour,
pub path_modification_behaviour: PathModificationBehavior,
}

impl ActivationVariables {
Expand All @@ -48,7 +48,7 @@ impl ActivationVariables {
Ok(Self {
conda_prefix: std::env::var("CONDA_PREFIX").ok().map(PathBuf::from),
path: None,
path_modification_behaviour: PathModificationBehaviour::Prepend,
path_modification_behaviour: PathModificationBehavior::Prepend,
})
}
}
Expand Down Expand Up @@ -485,7 +485,7 @@ mod tests {
use tempdir::TempDir;

#[cfg(unix)]
use crate::activation::PathModificationBehaviour;
use crate::activation::PathModificationBehavior;
use crate::shell::ShellEnum;

#[test]
Expand Down Expand Up @@ -601,7 +601,7 @@ mod tests {
#[cfg(unix)]
fn get_script<T: Shell>(
shell_type: T,
path_modification_behaviour: PathModificationBehaviour,
path_modification_behaviour: PathModificationBehavior,
) -> String
where
T: Clone,
Expand Down Expand Up @@ -631,25 +631,25 @@ mod tests {
#[test]
#[cfg(unix)]
fn test_activation_script_bash() {
let script = get_script(shell::Bash, PathModificationBehaviour::Append);
let script = get_script(shell::Bash, PathModificationBehavior::Append);
insta::assert_snapshot!("test_activation_script_bash_append", script);
let script = get_script(shell::Bash, PathModificationBehaviour::Replace);
let script = get_script(shell::Bash, PathModificationBehavior::Replace);
insta::assert_snapshot!("test_activation_script_bash_replace", script);
let script = get_script(shell::Bash, PathModificationBehaviour::Prepend);
let script = get_script(shell::Bash, PathModificationBehavior::Prepend);
insta::assert_snapshot!("test_activation_script_bash_prepend", script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_zsh() {
let script = get_script(shell::Zsh, PathModificationBehaviour::Append);
let script = get_script(shell::Zsh, PathModificationBehavior::Append);
insta::assert_snapshot!(script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_fish() {
let script = get_script(shell::Fish, PathModificationBehaviour::Append);
let script = get_script(shell::Fish, PathModificationBehavior::Append);
insta::assert_snapshot!(script);
}

Expand All @@ -658,36 +658,36 @@ mod tests {
fn test_activation_script_powershell() {
let script = get_script(
shell::PowerShell::default(),
PathModificationBehaviour::Append,
PathModificationBehavior::Append,
);
insta::assert_snapshot!("test_activation_script_powershell_append", script);
let script = get_script(
shell::PowerShell::default(),
PathModificationBehaviour::Prepend,
PathModificationBehavior::Prepend,
);
insta::assert_snapshot!("test_activation_script_powershell_prepend", script);
let script = get_script(
shell::PowerShell::default(),
PathModificationBehaviour::Replace,
PathModificationBehavior::Replace,
);
insta::assert_snapshot!("test_activation_script_powershell_replace", script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_cmd() {
let script = get_script(shell::CmdExe, PathModificationBehaviour::Append);
let script = get_script(shell::CmdExe, PathModificationBehavior::Append);
insta::assert_snapshot!("test_activation_script_cmd_append", script);
let script = get_script(shell::CmdExe, PathModificationBehaviour::Replace);
let script = get_script(shell::CmdExe, PathModificationBehavior::Replace);
insta::assert_snapshot!("test_activation_script_cmd_replace", script);
let script = get_script(shell::CmdExe, PathModificationBehaviour::Prepend);
let script = get_script(shell::CmdExe, PathModificationBehavior::Prepend);
insta::assert_snapshot!("test_activation_script_cmd_prepend", script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_xonsh() {
let script = get_script(shell::Xonsh, PathModificationBehaviour::Append);
let script = get_script(shell::Xonsh, PathModificationBehavior::Append);
insta::assert_snapshot!(script);
}

Expand Down
59 changes: 37 additions & 22 deletions crates/rattler_shell/src/shell/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! This module contains the [`Shell`] trait and implementations for various shells.

use crate::activation::PathModificationBehaviour;
use crate::activation::PathModificationBehavior;
use enum_dispatch::enum_dispatch;
use itertools::Itertools;
use rattler_conda_types::Platform;
Expand Down Expand Up @@ -55,7 +55,7 @@ pub trait Shell {
&self,
f: &mut impl Write,
paths: &[PathBuf],
modification_behaviour: PathModificationBehaviour,
modification_behaviour: PathModificationBehavior,
platform: &Platform,
) -> std::fmt::Result {
let mut paths_vec = paths
Expand All @@ -64,9 +64,9 @@ pub trait Shell {
.collect_vec();
// Replace, Append, or Prepend the path variable to the paths.
match modification_behaviour {
PathModificationBehaviour::Replace => (),
PathModificationBehaviour::Append => paths_vec.insert(0, self.format_env_var("PATH")),
PathModificationBehaviour::Prepend => paths_vec.push(self.format_env_var("PATH")),
PathModificationBehavior::Replace => (),
PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")),
PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")),
}
// Create the shell specific list of paths.
let paths_string = paths_vec.join(self.path_seperator(platform));
Expand Down Expand Up @@ -167,7 +167,7 @@ impl Shell for Bash {
&self,
f: &mut impl Write,
paths: &[PathBuf],
modification_behaviour: PathModificationBehaviour,
modification_behaviour: PathModificationBehavior,
platform: &Platform,
) -> std::fmt::Result {
// Put paths in a vector of the correct format.
Expand All @@ -193,9 +193,9 @@ impl Shell for Bash {

// Replace, Append, or Prepend the path variable to the paths.
match modification_behaviour {
PathModificationBehaviour::Replace => (),
PathModificationBehaviour::Prepend => paths_vec.push(self.format_env_var("PATH")),
PathModificationBehaviour::Append => paths_vec.insert(0, self.format_env_var("PATH")),
PathModificationBehavior::Replace => (),
PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")),
PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")),
}
// Create the shell specific list of paths.
let paths_string = paths_vec.join(self.path_seperator(platform));
Expand Down Expand Up @@ -422,13 +422,18 @@ impl Shell for Fish {
}
}

fn escape_backslashes(s: &str) -> String {
s.replace('\\', "\\\\")
}

/// A [`Shell`] implementation for the Bash shell.
#[derive(Debug, Clone, Copy, Default)]
pub struct NuShell;

impl Shell for NuShell {
fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result {
writeln!(f, "$env.{} = \"{}\"", env_var, value)
// escape backslashes for Windows (make them double backslashes)
writeln!(f, "$env.{} = \"{}\"", env_var, escape_backslashes(value))
}

fn unset_env_var(&self, f: &mut impl Write, env_var: &str) -> std::fmt::Result {
Expand All @@ -443,24 +448,24 @@ impl Shell for NuShell {
&self,
f: &mut impl Write,
paths: &[PathBuf],
modification_behaviour: PathModificationBehaviour,
modification_behaviour: PathModificationBehavior,
_platform: &Platform,
) -> std::fmt::Result {
let path = paths
.iter()
.map(|path| format!("\"{}\"", path.to_string_lossy().into_owned()))
.map(|path| escape_backslashes(&format!("\"{}\"", path.to_string_lossy().into_owned())))
.join(", ");

// Replace, Append, or Prepend the path variable to the paths.
match modification_behaviour {
PathModificationBehaviour::Replace => {
self.set_env_var(f, "PATH", &format!("[{}]", path))
PathModificationBehavior::Replace => {
writeln!(f, "$env.PATH = [{}]", path)
}
PathModificationBehaviour::Prepend => {
writeln!(f, "let-env PATH = ($env.PATH | prepend [{}])", path)
PathModificationBehavior::Prepend => {
writeln!(f, "$env.PATH = ($env.PATH | prepend [{}])", path)
}
PathModificationBehaviour::Append => {
writeln!(f, "let-env PATH = ($env.PATH | append [{}])", path)
PathModificationBehavior::Append => {
writeln!(f, "$env.PATH = ($env.PATH | append [{}])", path)
}
}
}
Expand Down Expand Up @@ -654,12 +659,16 @@ impl<T: Shell> ShellScript<T> {
}

/// Set the PATH environment variable to the given paths.
pub fn set_path(&mut self, paths: &[PathBuf]) -> &mut Self {
pub fn set_path(
&mut self,
paths: &[PathBuf],
path_modification_behavior: PathModificationBehavior,
) -> &mut Self {
self.shell
.set_path(
&mut self.contents,
paths,
PathModificationBehaviour::Prepend,
path_modification_behavior,
&self.platform,
)
.unwrap();
Expand Down Expand Up @@ -719,11 +728,17 @@ mod tests {
#[test]
fn test_path_seperator() {
let mut script = ShellScript::new(Bash, Platform::Linux64);
script.set_path(&[PathBuf::from("/foo"), PathBuf::from("/bar")]);
script.set_path(
&[PathBuf::from("/foo"), PathBuf::from("/bar")],
PathModificationBehavior::Prepend,
);
assert!(script.contents.contains("/foo:/bar"));

let mut script = ShellScript::new(Bash, Platform::Win64);
script.set_path(&[PathBuf::from("/foo"), PathBuf::from("/bar")]);
script.set_path(
&[PathBuf::from("/foo"), PathBuf::from("/bar")],
PathModificationBehavior::Prepend,
);
assert!(script.contents.contains("/foo;/bar"));
}
}
12 changes: 6 additions & 6 deletions py-rattler/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::error::PyRattlerError;
use crate::platform::PyPlatform;
use pyo3::{exceptions::PyValueError, pyclass, pymethods, FromPyObject, PyAny, PyResult};
use rattler_shell::{
activation::{ActivationResult, ActivationVariables, Activator, PathModificationBehaviour},
activation::{ActivationResult, ActivationVariables, Activator, PathModificationBehavior},
shell::{Bash, CmdExe, Fish, PowerShell, Xonsh, Zsh},
};
use std::path::{Path, PathBuf};
Expand All @@ -23,12 +23,12 @@ impl From<ActivationVariables> for PyActivationVariables {
#[repr(transparent)]
pub struct Wrap<T>(pub T);

impl FromPyObject<'_> for Wrap<PathModificationBehaviour> {
impl FromPyObject<'_> for Wrap<PathModificationBehavior> {
fn extract(ob: &PyAny) -> PyResult<Self> {
let parsed = match ob.extract::<&str>()? {
"prepend" => PathModificationBehaviour::Prepend,
"append" => PathModificationBehaviour::Append,
"replace" => PathModificationBehaviour::Replace,
"prepend" => PathModificationBehavior::Prepend,
"append" => PathModificationBehavior::Append,
"replace" => PathModificationBehavior::Replace,
v => {
return Err(PyValueError::new_err(format!(
"keep must be one of {{'prepend', 'append', 'replace'}}, got {v}",
Expand All @@ -46,7 +46,7 @@ impl PyActivationVariables {
pub fn __init__(
conda_prefix: Option<PathBuf>,
path: Option<Vec<PathBuf>>,
path_modification_behaviour: Wrap<PathModificationBehaviour>,
path_modification_behaviour: Wrap<PathModificationBehavior>,
) -> Self {
let activation_vars = ActivationVariables {
conda_prefix,
Expand Down