Skip to content

Changed all impls to use string slices for better perf #111

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
59 changes: 31 additions & 28 deletions src/bundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::path::Path;
/// A simple wrapper around the `bundle` command.
pub struct Bundler {
pub working_dir: String,
envs: Vec<(String, String)>,
command_executor: Box<dyn CommandExecutor>,
}

Expand All @@ -14,14 +13,9 @@ impl Bundler {
/// # Arguments
/// * `working_dir` - The working directory where `bundle` commands should be executed.
/// * `command_executor` - An executor for `bundle` commands.
pub fn new(
working_dir: String,
envs: Vec<(String, String)>,
command_executor: Box<dyn CommandExecutor>,
) -> Self {
pub fn new(working_dir: String, command_executor: Box<dyn CommandExecutor>) -> Self {
Bundler {
working_dir,
envs,
command_executor,
}
}
Expand All @@ -33,31 +27,36 @@ impl Bundler {
///
/// # Returns
/// A `Result` containing the version string if successful, or an error message.
pub fn installed_gem_version(&self, name: &str) -> Result<String, String> {
let args = vec!["--version".into(), name.into()];

self.execute_bundle_command("info".into(), args)
pub fn installed_gem_version(
&self,
name: &str,
envs: &[(&str, &str)],
) -> Result<String, String> {
let args = &["--version", name];

self.execute_bundle_command("info", args, envs)
}

fn execute_bundle_command(&self, cmd: String, args: Vec<String>) -> Result<String, String> {
fn execute_bundle_command(
&self,
cmd: &str,
args: &[&str],
envs: &[(&str, &str)],
) -> Result<String, String> {
let bundle_gemfile_path = Path::new(&self.working_dir).join("Gemfile");
let bundle_gemfile = bundle_gemfile_path
.to_str()
.ok_or_else(|| "Invalid path to Gemfile".to_string())?;

let full_args: Vec<String> = std::iter::once(cmd).chain(args).collect();
let command_envs: Vec<(String, String)> = self
.envs
let full_args: Vec<&str> = std::iter::once(cmd).chain(args.iter().copied()).collect();
let command_envs: Vec<(&str, &str)> = envs
.iter()
.cloned()
.chain(std::iter::once((
"BUNDLE_GEMFILE".to_string(),
bundle_gemfile.to_string(),
)))
.chain(std::iter::once(("BUNDLE_GEMFILE", bundle_gemfile)))
.collect();

self.command_executor
.execute("bundle", full_args, command_envs)
.execute("bundle", &full_args, &command_envs)
.and_then(|output| match output.status {
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).to_string()),
Some(status) => {
Expand Down Expand Up @@ -129,8 +128,8 @@ mod tests {
fn execute(
&self,
command_name: &str,
args: Vec<String>,
envs: Vec<(String, String)>,
args: &[&str],
envs: &[(&str, &str)],
) -> Result<Output, String> {
let mut config = self.config.borrow_mut();

Expand All @@ -141,6 +140,10 @@ mod tests {
assert_eq!(&args, expected_args, "Mock: Args mismatch");
}
if let Some(expected_envs) = &config.expected_envs {
let envs: Vec<(String, String)> = envs
.iter()
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect();
assert_eq!(&envs, expected_envs, "Mock: Env mismatch");
}

Expand Down Expand Up @@ -173,9 +176,9 @@ mod tests {
#[test]
fn test_installed_gem_version_success() {
let mock_executor = create_mock_executor_for_success("8.0.0", "test_dir", "rails");
let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
let bundler = Bundler::new("test_dir".into(), Box::new(mock_executor));
let version = bundler
.installed_gem_version("rails")
.installed_gem_version("rails", &[])
.expect("Expected successful version");
assert_eq!(version, "8.0.0", "Installed gem version should match");
}
Expand All @@ -198,8 +201,8 @@ mod tests {
}),
);

let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
let result = bundler.installed_gem_version(gem_name);
let bundler = Bundler::new("test_dir".into(), Box::new(mock_executor));
let result = bundler.installed_gem_version(gem_name, &[]);

assert!(
result.is_err(),
Expand Down Expand Up @@ -230,8 +233,8 @@ mod tests {
Err(specific_error_msg.to_string()),
);

let bundler = Bundler::new("test_dir".into(), vec![], Box::new(mock_executor));
let result = bundler.installed_gem_version(gem_name);
let bundler = Bundler::new("test_dir".into(), Box::new(mock_executor));
let result = bundler.installed_gem_version(gem_name, &[]);

assert!(result.is_err(), "Expected error from executor failure");
assert_eq!(
Expand Down
14 changes: 9 additions & 5 deletions src/command_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,26 @@ pub trait CommandExecutor {
fn execute(
&self,
cmd: &str,
args: Vec<String>,
envs: Vec<(String, String)>,
args: &[&str],
envs: &[(&str, &str)],
) -> zed::Result<zed::process::Output>;
}

/// An implementation of `CommandExecutor` that executes commands
/// using the `zed_extension_api::Command`.
#[derive(Clone)]
pub struct RealCommandExecutor;

impl CommandExecutor for RealCommandExecutor {
fn execute(
&self,
cmd: &str,
args: Vec<String>,
envs: Vec<(String, String)>,
args: &[&str],
envs: &[(&str, &str)],
) -> zed::Result<zed::process::Output> {
zed::Command::new(cmd).args(args).envs(envs).output()
zed::Command::new(cmd)
.args(args.iter().copied())
.envs(envs.iter().copied())
.output()
}
}
54 changes: 28 additions & 26 deletions src/gemset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ impl Gemset {
}

/// Returns the full path to a gem binary executable.
pub fn gem_bin_path(&self, bin_name: impl Into<String>) -> Result<String, String> {
let bin_name = bin_name.into();
pub fn gem_bin_path(&self, bin_name: &str) -> Result<String, String> {
let path = std::path::Path::new(&self.gem_home)
.join("bin")
.join(&bin_name);
.join(bin_name);

path.to_str()
.map(ToString::to_string)
Expand All @@ -35,21 +34,21 @@ impl Gemset {
}

pub fn install_gem(&self, name: &str) -> Result<(), String> {
let args = vec![
"--no-user-install".to_string(),
"--no-format-executable".to_string(),
"--no-document".to_string(),
name.into(),
let args = &[
"--no-user-install",
"--no-format-executable",
"--no-document",
name,
];

self.execute_gem_command("install".into(), args)
self.execute_gem_command("install", args)
.map_err(|e| format!("Failed to install gem '{}': {}", name, e))?;

Ok(())
}

pub fn update_gem(&self, name: &str) -> Result<(), String> {
self.execute_gem_command("update".into(), vec![name.into()])
self.execute_gem_command("update", &[name])
.map_err(|e| format!("Failed to update gem '{}': {}", name, e))?;
Ok(())
}
Expand All @@ -58,8 +57,8 @@ impl Gemset {
let re = Regex::new(r"^(\S+) \((.+)\)$")
.map_err(|e| format!("Failed to compile regex: {}", e))?;

let args = vec!["--exact".to_string(), name.into()];
let output_str = self.execute_gem_command("list".into(), args)?;
let args = &["--exact", name];
let output_str = self.execute_gem_command("list", args)?;

for line in output_str.lines() {
let captures = match re.captures(line) {
Expand All @@ -78,23 +77,22 @@ impl Gemset {
}

pub fn is_outdated_gem(&self, name: &str) -> Result<bool, String> {
self.execute_gem_command("outdated".into(), vec![])
.map(|output| {
output
.lines()
.any(|line| line.split_whitespace().next().is_some_and(|n| n == name))
})
self.execute_gem_command("outdated", &[]).map(|output| {
output
.lines()
.any(|line| line.split_whitespace().next().is_some_and(|n| n == name))
})
}

fn execute_gem_command(&self, cmd: String, args: Vec<String>) -> Result<String, String> {
let full_args: Vec<String> = std::iter::once(cmd)
.chain(std::iter::once("--norc".to_string()))
.chain(args)
fn execute_gem_command(&self, cmd: &str, args: &[&str]) -> Result<String, String> {
let full_args: Vec<&str> = std::iter::once(cmd)
.chain(std::iter::once("--norc"))
.chain(args.iter().copied())
.collect();
let command_envs = vec![("GEM_HOME".to_string(), self.gem_home.clone())];
let command_envs = &[("GEM_HOME", self.gem_home.as_str())];

self.command_executor
.execute("gem", full_args, command_envs)
.execute("gem", &full_args, command_envs)
.and_then(|output| match output.status {
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).to_string()),
Some(status) => {
Expand Down Expand Up @@ -166,8 +164,8 @@ mod tests {
fn execute(
&self,
command_name: &str,
args: Vec<String>,
envs: Vec<(String, String)>,
args: &[&str],
envs: &[(&str, &str)],
) -> Result<Output, String> {
let mut config = self.config.borrow_mut();

Expand All @@ -178,6 +176,10 @@ mod tests {
assert_eq!(&args, expected_args, "Mock: Args mismatch");
}
if let Some(expected_envs) = &config.expected_envs {
let envs: Vec<(String, String)> = envs
.iter()
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect();
assert_eq!(&envs, expected_envs, "Mock: Env mismatch");
}

Expand Down
16 changes: 9 additions & 7 deletions src/language_servers/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,14 @@ pub trait LanguageServer {
return self.try_find_on_path_or_extension_gemset(language_server_id, worktree);
}

let bundler = Bundler::new(
worktree.root_path(),
worktree.shell_env(),
Box::new(RealCommandExecutor),
);
match bundler.installed_gem_version(Self::GEM_NAME) {
let bundler = Bundler::new(worktree.root_path(), Box::new(RealCommandExecutor));
let shell_env = worktree.shell_env();
let env_vars: Vec<(&str, &str)> = shell_env
.iter()
.map(|(key, value)| (key.as_str(), value.as_str()))
.collect();

match bundler.installed_gem_version(Self::GEM_NAME, &env_vars) {
Ok(_version) => {
let bundle_path = worktree
.which("bundle")
Expand All @@ -183,7 +185,7 @@ pub trait LanguageServer {
.chain(self.get_executable_args(worktree))
.collect(),
),
env: Some(worktree.shell_env()),
env: Some(shell_env),
})
}
Err(_e) => self.try_find_on_path_or_extension_gemset(language_server_id, worktree),
Expand Down
14 changes: 7 additions & 7 deletions src/language_servers/sorbet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@ impl LanguageServer for Sorbet {
.lsp_binary_settings(Self::SERVER_ID)
.unwrap_or_default();

let default_args = vec![
"tc".to_string(),
"--lsp".to_string(),
"--enable-experimental-lsp-document-highlight".to_string(),
];

// test if sorbet/config is present
match worktree.read_text_file("sorbet/config") {
Ok(_) => {
// Config file exists, prefer custom arguments if available.
binary_settings
.and_then(|bs| bs.arguments)
.unwrap_or(default_args)
.unwrap_or_else(|| {
vec![
"tc".to_string(),
"--lsp".to_string(),
"--enable-experimental-lsp-document-highlight".to_string(),
]
})
}
Err(_) => {
// gross, but avoid sorbet errors in a non-sorbet
Expand Down