Skip to content
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
9 changes: 3 additions & 6 deletions src/debugger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ use zed_extension_api::{
use crate::{
config::get_java_debug_jar,
lsp::LspWrapper,
util::{
create_path_if_not_exists, get_curr_dir, path_to_quoted_string,
should_use_local_or_download,
},
util::{create_path_if_not_exists, get_curr_dir, path_to_string, should_use_local_or_download},
};

#[derive(Serialize, Deserialize, Debug)]
Expand Down Expand Up @@ -155,7 +152,7 @@ impl Debugger {

download_file(
JAVA_DEBUG_PLUGIN_FORK_URL,
&path_to_quoted_string(jar_path.clone())?,
&path_to_string(jar_path.clone())?,
DownloadedFileType::Uncompressed,
)
.map_err(|err| {
Expand Down Expand Up @@ -259,7 +256,7 @@ impl Debugger {

download_file(
url.as_str(),
&path_to_quoted_string(&jar_path)?,
&path_to_string(&jar_path)?,
DownloadedFileType::Uncompressed,
)
.map_err(|err| format!("Failed to download {url} {err}"))?;
Expand Down
9 changes: 4 additions & 5 deletions src/java.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::{
try_to_fetch_and_install_latest_lombok,
},
lsp::LspWrapper,
util::path_to_quoted_string,
util::path_to_string,
};

const PROXY_FILE: &str = include_str!("proxy.mjs");
Expand Down Expand Up @@ -279,17 +279,16 @@ impl Extension for Java {
"--input-type=module".to_string(),
"-e".to_string(),
PROXY_FILE.to_string(),
path_to_quoted_string(current_dir.clone())?,
path_to_string(current_dir.clone())?,
];

// Add lombok as javaagent if settings.java.jdt.ls.lombokSupport.enabled is true
let lombok_jvm_arg = if is_lombok_enabled(&configuration) {
let lombok_jar_path =
self.lombok_jar_path(language_server_id, &configuration, worktree)?;
let canonical_lombok_jar_path =
path_to_quoted_string(current_dir.join(lombok_jar_path))?;
let canonical_lombok_jar_path = path_to_string(current_dir.join(lombok_jar_path))?;

Some(format!("-javaagent:{canonical_lombok_jar_path}"))
Some(format!("-javaagent:{}", canonical_lombok_jar_path))
} else {
None
};
Expand Down
4 changes: 2 additions & 2 deletions src/jdk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use zed_extension_api::{
set_language_server_installation_status,
};

use crate::util::{get_curr_dir, path_to_quoted_string, remove_all_files_except};
use crate::util::{get_curr_dir, path_to_string, remove_all_files_except};

// Errors
const JDK_DIR_ERROR: &str = "Failed to read into JDK install directory";
Expand Down Expand Up @@ -79,7 +79,7 @@ pub fn try_to_fetch_and_install_latest_jdk(

download_file(
build_corretto_url(&version, &platform, &arch).as_str(),
path_to_quoted_string(install_path.clone())?.as_str(),
path_to_string(install_path.clone())?.as_str(),
match zed::current_platform().0 {
Os::Windows => DownloadedFileType::Zip,
_ => DownloadedFileType::GzipTar,
Expand Down
16 changes: 8 additions & 8 deletions src/jdtls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
jdk::try_to_fetch_and_install_latest_jdk,
util::{
create_path_if_not_exists, get_curr_dir, get_java_exec_name, get_java_executable,
get_java_major_version, get_latest_versions_from_tag, path_to_quoted_string,
get_java_major_version, get_latest_versions_from_tag, path_to_string,
remove_all_files_except, should_use_local_or_download,
},
};
Expand Down Expand Up @@ -65,14 +65,14 @@ pub fn build_jdtls_launch_args(
let jdtls_data_path = get_jdtls_data_path(worktree)?;

let mut args = vec![
path_to_quoted_string(java_executable)?,
path_to_string(java_executable)?,
"-Declipse.application=org.eclipse.jdt.ls.core.id1".to_string(),
"-Dosgi.bundles.defaultStartLevel=4".to_string(),
"-Declipse.product=org.eclipse.jdt.ls.core.product".to_string(),
"-Dosgi.checkConfiguration=true".to_string(),
format!(
"-Dosgi.sharedConfiguration.area={}",
path_to_quoted_string(shared_config_path)?
path_to_string(shared_config_path)?
),
"-Dosgi.sharedConfiguration.area.readOnly=true".to_string(),
"-Dosgi.configuration.cascaded=true".to_string(),
Expand All @@ -86,9 +86,9 @@ pub fn build_jdtls_launch_args(
args.extend(jvm_args);
args.extend(vec![
"-jar".to_string(),
path_to_quoted_string(jar_path)?,
path_to_string(jar_path)?,
"-data".to_string(),
path_to_quoted_string(jdtls_data_path)?,
path_to_string(jdtls_data_path)?,
]);
if java_major_version >= 24 {
args.push("-Djdk.xml.maxGeneralEntitySizeLimit=0".to_string());
Expand Down Expand Up @@ -195,10 +195,10 @@ pub fn try_to_fetch_and_install_latest_jdtls(
&format!(
"https://www.eclipse.org/downloads/download.php?file=/jdtls/milestones/{latest_version}/{latest_version_build}"
),
path_to_quoted_string(build_path.clone())?.as_str(),
path_to_string(build_path.clone())?.as_str(),
DownloadedFileType::GzipTar,
)?;
make_file_executable(path_to_quoted_string(binary_path)?.as_str())?;
make_file_executable(path_to_string(binary_path)?.as_str())?;

// ...and delete other versions
let _ = remove_all_files_except(prefix, build_directory.as_str());
Expand Down Expand Up @@ -241,7 +241,7 @@ pub fn try_to_fetch_and_install_latest_lombok(
create_path_if_not_exists(prefix)?;
download_file(
&format!("https://projectlombok.org/downloads/{jar_name}"),
path_to_quoted_string(jar_path.clone())?.as_str(),
path_to_string(jar_path.clone())?.as_str(),
DownloadedFileType::Uncompressed,
)?;

Expand Down
17 changes: 8 additions & 9 deletions src/proxy.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ const args = process.argv.slice(3);
const PROXY_ID = Buffer.from(process.cwd().replace(/\/+$/, "")).toString("hex");
const PROXY_HTTP_PORT_FILE = join(workdir, "proxy", PROXY_ID);
const isWindows = process.platform === "win32";
const command = isWindows ? `"${bin}"` : bin;
const command = (isWindows && bin.endsWith(".bat")) ? `"${bin}"` : bin;

const lsp = spawn(command, args, {
shell: isWindows,
detached: false
shell: (isWindows && bin.endsWith(".bat")) ? true : false,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary ternary expression here. But if we always have to predicate isWindows with bin.endsWith(".bat"), we might factor that out.

detached: false,
});

function cleanup() {
Expand All @@ -44,19 +44,18 @@ function cleanup() {
// Windows: Use taskkill to kill the process tree (cmd.exe + the child)
// /T = Tree kill (child processes), /F = Force
exec(`taskkill /pid ${lsp.pid} /T /F`);
}
else {
lsp.kill('SIGTERM');
} else {
lsp.kill("SIGTERM");
setTimeout(() => {
if (!lsp.killed && lsp.exitCode === null) {
lsp.kill('SIGKILL');
lsp.kill("SIGKILL");
}
}, 1000);
}
}

// Handle graceful IDE shutdown via stdin close
process.stdin.on('end', () => {
process.stdin.on("end", () => {
cleanup();
process.exit(0);
});
Expand All @@ -71,7 +70,7 @@ setInterval(() => {
} catch (e) {
// On Windows, checking a process you don't own might throw EPERM.
// We only want to kill if the error is ESRCH (No Such Process).
if (e.code === 'ESRCH') {
if (e.code === "ESRCH") {
cleanup();
process.exit(0);
}
Expand Down
64 changes: 7 additions & 57 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ pub fn get_java_exec_name() -> String {
/// * [`java_executable`] can't be converted into a String
/// * No major version can be determined
pub fn get_java_major_version(java_executable: &PathBuf) -> zed::Result<u32> {
let program =
path_to_quoted_string(java_executable).map_err(|_| JAVA_EXEC_ERROR.to_string())?;
let program = path_to_string(java_executable).map_err(|_| JAVA_EXEC_ERROR.to_string())?;
let output_bytes = Command::new(program).arg("-version").output()?.stderr;
let output = String::from_utf8(output_bytes).map_err(|e| e.to_string())?;

Expand Down Expand Up @@ -247,45 +246,25 @@ fn get_tag_at(github_tags: &Value, index: usize) -> Option<&str> {
})
}

/// Formats a path string with platform-specific quoting.
///
/// On Windows, wraps the path in double quotes for shell mode compatibility.
/// On Unix, returns the path unquoted since spawn() treats quotes as literals.
fn format_path_for_os(path_str: String, os: Os) -> String {
if os == Os::Windows {
format!("\"{}\"", path_str)
} else {
path_str
}
}

/// Converts a [`Path`] into a [`String`], with platform-specific quoting.
///
/// On Windows, the path is wrapped in double quotes (e.g., `"C:\path\to\file"`)
/// for compatibility with shell mode. On Unix-like systems, the path is returned
/// unquoted, as the proxy uses `spawn()` with `shell: false` which treats quotes
/// as literal filename characters, causing "No such file or directory" errors.
/// Converts a [`Path`] into a [`String`].
///
/// # Arguments
///
/// * `path` - The path of type `AsRef<Path>` to be converted.
///
/// # Returns
///
/// Returns a `String` representing the path, quoted on Windows, unquoted on Unix.
/// Returns a `String` representing the path.
///
/// # Errors
///
/// This function will return an error when the string conversion fails
pub fn path_to_quoted_string<P: AsRef<Path>>(path: P) -> zed::Result<String> {
let path_str = path
.as_ref()
/// This function will return an error when the string conversion fails.
pub fn path_to_string<P: AsRef<Path>>(path: P) -> zed::Result<String> {
path.as_ref()
.to_path_buf()
.into_os_string()
.into_string()
.map_err(|_| PATH_TO_STR_ERROR.to_string())?;

Ok(format_path_for_os(path_str, current_platform().0))
.map_err(|_| PATH_TO_STR_ERROR.to_string())
}

/// Remove all files or directories that aren't equal to [`filename`].
Expand Down Expand Up @@ -366,32 +345,3 @@ pub fn should_use_local_or_download(
CheckUpdates::Always => Ok(None),
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_format_path_for_os_windows() {
let path = "C:\\Users\\User Name\\Projects\\zed-extension-java".to_string();
let result = format_path_for_os(path, Os::Windows);
assert_eq!(
result,
"\"C:\\Users\\User Name\\Projects\\zed-extension-java\""
);
}

#[test]
fn test_format_path_for_os_unix() {
let path = "/home/username/Projects/zed extension java".to_string();
let result = format_path_for_os(path, Os::Mac);
assert_eq!(result, "/home/username/Projects/zed extension java");
}

#[test]
fn test_format_path_for_os_linux() {
let path = "/home/username/Projects/zed extension java".to_string();
let result = format_path_for_os(path, Os::Linux);
assert_eq!(result, "/home/username/Projects/zed extension java");
}
}