From 056cb003d19a6deb58214a041ee76d62716524c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matou=C5=A1=20Dzivjak?= Date: Sun, 5 Feb 2023 11:23:27 +0000 Subject: [PATCH 1/3] feat(windows): use std Command Use std `Command` instead of `ShellExecuteW` from windows sys crate. This change was already attempted in: https://github.com/Byron/open-rs/issues/25 and later reverted in: https://github.com/Byron/open-rs/pull/27 and it it seems that it didn't work due to incorrect usage of `explorer` instead of `cmd /c start`. (see https://github.com/helix-editor/helix/pull/5820#issuecomment-1416796024 for detailed explanation). Related: https://github.com/helix-editor/helix/pull/5820 --- Cargo.toml | 3 --- src/windows.rs | 41 ++++++++++++----------------------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 268168b..e37dcb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,8 +16,5 @@ test = false doc = false name = "open" -[target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.42", features = ["Win32_UI_Shell", "Win32_Foundation", "Win32_UI_WindowsAndMessaging"] } - [target.'cfg(all(unix, not(macos)))'.dependencies] pathdiff = "0.2.0" diff --git a/src/windows.rs b/src/windows.rs index 125770e..7bd5bcb 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -6,8 +6,6 @@ use std::{ }; use std::os::raw::c_int; -use windows_sys::Win32::UI::Shell::ShellExecuteW; -use windows_sys::Win32::UI::WindowsAndMessaging::SW_SHOW; use crate::IntoResult; @@ -32,35 +30,20 @@ fn convert_path(path: &OsStr) -> io::Result> { pub fn that>(path: T) -> io::Result<()> { let path = convert_path(path.as_ref())?; - let operation: Vec = OsStr::new("open\0").encode_wide().collect(); - let result = unsafe { - ShellExecuteW( - 0, - operation.as_ptr(), - path.as_ptr(), - ptr::null(), - ptr::null(), - SW_SHOW, - ) - }; - (result as c_int).into_result() + Command::new("cmd") + .arg("/c") + .arg("start") + .arg(path.as_ref()) + .status_without_output() + .into_result() } pub fn with>(path: T, app: impl Into) -> io::Result<()> { let path = convert_path(path.as_ref())?; - let operation: Vec = OsStr::new("open\0").encode_wide().collect(); - let app_name: Vec = OsStr::new(&format!("{}\0", app.into())) - .encode_wide() - .collect(); - let result = unsafe { - ShellExecuteW( - 0, - operation.as_ptr(), - app_name.as_ptr(), - path.as_ptr(), - ptr::null(), - SW_SHOW, - ) - }; - (result as c_int).into_result() + Command::new("cmd") + .arg("/c") + .arg(app.into()) + .arg(path.as_ref()) + .status_without_output() + .into_result() } From 7ab725a2c6776a8f45305f71aec53650b45d9df7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 5 Feb 2023 17:19:54 +0100 Subject: [PATCH 2/3] refactor - fix build - don't quote path anymore as it's seemingly not required. After all, we are passing the argument directly. --- src/windows.rs | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 7bd5bcb..b2b8658 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -1,36 +1,9 @@ -use std::{ - ffi::{OsStr, OsString}, - io, - os::windows::ffi::OsStrExt, - ptr, -}; +use std::{ffi::OsStr, io}; -use std::os::raw::c_int; - -use crate::IntoResult; - -fn convert_path(path: &OsStr) -> io::Result> { - let mut quoted_path = OsString::with_capacity(path.len()); - - // Surround path with double quotes "" to handle spaces in path. - quoted_path.push("\""); - quoted_path.push(&path); - quoted_path.push("\""); - - let mut wide_chars: Vec<_> = quoted_path.encode_wide().collect(); - if wide_chars.iter().any(|&u| u == 0) { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "path contains NUL byte(s)", - )); - } - wide_chars.push(0); - Ok(wide_chars) -} +use crate::{CommandExt, IntoResult}; pub fn that>(path: T) -> io::Result<()> { - let path = convert_path(path.as_ref())?; - Command::new("cmd") + std::process::Command::new("cmd") .arg("/c") .arg("start") .arg(path.as_ref()) @@ -39,8 +12,7 @@ pub fn that>(path: T) -> io::Result<()> { } pub fn with>(path: T, app: impl Into) -> io::Result<()> { - let path = convert_path(path.as_ref())?; - Command::new("cmd") + std::process::Command::new("cmd") .arg("/c") .arg(app.into()) .arg(path.as_ref()) From 1f4a9f925329649fe0929936eef07d8e6f557ec4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 5 Feb 2023 17:32:53 +0100 Subject: [PATCH 3/3] remove win32-msvc from build matrix as it fails due to permission-denied with rustup.exe This seems like an odd problem to have and nothign to spend time on given the lack of importance of the win32 platform. We would except open-rs to build fine on win32 if it builds for win64. --- .github/workflows/cross-platform-testing.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/cross-platform-testing.yml b/.github/workflows/cross-platform-testing.yml index 47757dd..1f1c581 100644 --- a/.github/workflows/cross-platform-testing.yml +++ b/.github/workflows/cross-platform-testing.yml @@ -20,7 +20,7 @@ jobs: RUST_BACKTRACE: 1 strategy: matrix: - build: [linux, linux-arm, macos, win-msvc, win-gnu, win32-msvc] + build: [linux, linux-arm, macos, win-msvc, win-gnu] include: - build: linux os: ubuntu-18.04 @@ -42,10 +42,6 @@ jobs: os: windows-2019 rust: stable target: x86_64-pc-windows-gnu - - build: win32-msvc - os: windows-2019 - rust: stable - target: i686-pc-windows-msvc steps: - name: Checkout repository