From f632919134414e31ffd3ae9b5732d673deb0adf5 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 2 Jan 2020 11:49:27 +0000 Subject: [PATCH] Clean up Windows PTY string handling Removes widestring and dunce dependencies, reduces some code duplication and corrects a few typos. --- Cargo.lock | 15 ---------- alacritty/src/cli.rs | 12 ++++---- alacritty_terminal/Cargo.toml | 2 -- alacritty_terminal/src/config/mod.rs | 6 ++-- alacritty_terminal/src/panic.rs | 12 ++------ alacritty_terminal/src/tty/mod.rs | 2 +- alacritty_terminal/src/tty/unix.rs | 2 +- alacritty_terminal/src/tty/windows/conpty.rs | 27 ++++++------------ alacritty_terminal/src/tty/windows/mod.rs | 21 +++++++++++++- alacritty_terminal/src/tty/windows/winpty.rs | 20 ++++--------- winpty/Cargo.toml | 1 - winpty/src/lib.rs | 8 ------ winpty/src/windows.rs | 30 ++++++++++++-------- 13 files changed, 65 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ca13d65..61070efb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,7 +58,6 @@ dependencies = [ "base64 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "copypasta 0.6.1", - "dunce 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "font 0.1.0", "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -79,7 +78,6 @@ dependencies = [ "unicode-width 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "url 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "vte 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "widestring 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "winpty 0.1.0", ] @@ -488,11 +486,6 @@ name = "dtoa" version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -[[package]] -name = "dunce" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" - [[package]] name = "dwrote" version = "0.9.0" @@ -2242,11 +2235,6 @@ dependencies = [ "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "widestring" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" - [[package]] name = "winapi" version = "0.2.8" @@ -2328,7 +2316,6 @@ dependencies = [ "http_req 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "named_pipe 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "widestring 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "winpty-sys 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "zip 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2469,7 +2456,6 @@ dependencies = [ "checksum dlib 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "77e51249a9d823a4cb79e3eca6dcd756153e8ed0157b6c04775d04bf1b13b76a" "checksum downcast-rs 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "52ba6eb47c2131e784a38b726eb54c1e1484904f013e576a25354d0124161af6" "checksum dtoa 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "ea57b42383d091c85abcc2706240b94ab2a8fa1fc81c10ff23c4de06e2a90b5e" -"checksum dunce 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d0ad6bf6a88548d1126045c413548df1453d9be094a8ab9fd59bf1fdd338da4f" "checksum dwrote 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0bd1369e02db5e9b842a9b67bce8a2fcc043beafb2ae8a799dd482d46ea1ff0d" "checksum embed-resource 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bbaba4684ab0af1cbb3ef0b1f540ddc4b57b31940c920ea594efe09ab86e2a6c" "checksum env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)" = "15b0a4d2e39f8420210be8b27eeda28029729e2fd4291019455016c348240c38" @@ -2671,7 +2657,6 @@ dependencies = [ "checksum wayland-scanner 0.23.6 (registry+https://github.com/rust-lang/crates.io-index)" = "93b02247366f395b9258054f964fe293ddd019c3237afba9be2ccbe9e1651c3d" "checksum wayland-sys 0.23.6 (registry+https://github.com/rust-lang/crates.io-index)" = "d94e89a86e6d6d7c7c9b19ebf48a03afaac4af6bc22ae570e9a24124b75358f4" "checksum which 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)" = "e84a603e7e0b1ce1aa1ee2b109c7be00155ce52df5081590d1ffb93f4f515cb2" -"checksum widestring 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "effc0e4ff8085673ea7b9b2e3c73f6bd4d118810c9009ed8f1e16bd96c331db6" "checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a" "checksum winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "8093091eeb260906a183e6ae1abdba2ef5ef2257a21801128899c3fc699229c6" "checksum winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc" diff --git a/alacritty/src/cli.rs b/alacritty/src/cli.rs index e3fb8dcf..6f7f8a68 100644 --- a/alacritty/src/cli.rs +++ b/alacritty/src/cli.rs @@ -254,12 +254,12 @@ impl Options { } pub fn into_config(self, mut config: Config) -> Config { - config.set_live_config_reload( - self.live_config_reload.unwrap_or_else(|| config.live_config_reload()), - ); - config.set_working_directory( - self.working_dir.or_else(|| config.working_directory().to_owned()), - ); + if let Some(lcr) = self.live_config_reload { + config.set_live_config_reload(lcr); + } + if let Some(wd) = self.working_dir { + config.set_working_directory(Some(wd)); + } config.shell = self.command.or(config.shell); config.hold = self.hold; diff --git a/alacritty_terminal/Cargo.toml b/alacritty_terminal/Cargo.toml index 385c3cbd..947d94e4 100644 --- a/alacritty_terminal/Cargo.toml +++ b/alacritty_terminal/Cargo.toml @@ -34,12 +34,10 @@ signal-hook = { version = "0.1", features = ["mio-support"] } winpty = { path = "../winpty" } mio-named-pipes = "0.1" miow = "0.3" -dunce = "1.0" winapi = { version = "0.3.7", features = [ "impl-default", "basetsd", "libloaderapi", "minwindef", "ntdef", "processthreadsapi", "winbase", "wincon", "wincontypes", "winerror", "winnt", "winuser", ]} -widestring = "0.4" mio-anonymous-pipes = "0.1" [target.'cfg(target_os = "macos")'.dependencies] diff --git a/alacritty_terminal/src/config/mod.rs b/alacritty_terminal/src/config/mod.rs index 3c8a85a4..f3257b7b 100644 --- a/alacritty_terminal/src/config/mod.rs +++ b/alacritty_terminal/src/config/mod.rs @@ -15,7 +15,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::fmt::Display; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use log::error; use serde::{Deserialize, Deserializer}; @@ -209,8 +209,8 @@ impl Config { } #[inline] - pub fn working_directory(&self) -> &Option { - &self.working_directory + pub fn working_directory(&self) -> Option<&Path> { + self.working_directory.as_ref().map(|path_buf| path_buf.as_path()) } #[inline] diff --git a/alacritty_terminal/src/panic.rs b/alacritty_terminal/src/panic.rs index 4d3524ed..7f19c59a 100644 --- a/alacritty_terminal/src/panic.rs +++ b/alacritty_terminal/src/panic.rs @@ -13,6 +13,8 @@ // limitations under the License. // //! ANSI Terminal Stream Parsing +#[cfg(windows)] +use crate::tty::windows::win32_string; // Use the default behavior of the other platforms. #[cfg(not(windows))] @@ -41,13 +43,3 @@ pub fn attach_handler() { } })); } - -// Converts the string slice into a Windows-standard representation for "W"- -// suffixed function variants, which accept UTF-16 encoded string values. -#[cfg(windows)] -fn win32_string(value: &str) -> Vec { - use std::ffi::OsStr; - use std::iter::once; - use std::os::windows::ffi::OsStrExt; - OsStr::new(value).encode_wide().chain(once(0)).collect() -} diff --git a/alacritty_terminal/src/tty/mod.rs b/alacritty_terminal/src/tty/mod.rs index 40d019f5..425ec4b0 100644 --- a/alacritty_terminal/src/tty/mod.rs +++ b/alacritty_terminal/src/tty/mod.rs @@ -26,7 +26,7 @@ mod unix; pub use self::unix::*; #[cfg(windows)] -mod windows; +pub mod windows; #[cfg(windows)] pub use self::windows::*; diff --git a/alacritty_terminal/src/tty/unix.rs b/alacritty_terminal/src/tty/unix.rs index 9419ead0..ab34d33a 100644 --- a/alacritty_terminal/src/tty/unix.rs +++ b/alacritty_terminal/src/tty/unix.rs @@ -209,7 +209,7 @@ pub fn new(config: &Config, size: &SizeInfo, window_id: Option) -> // Handle set working directory option if let Some(ref dir) = config.working_directory() { - builder.current_dir(dir.as_path()); + builder.current_dir(dir); } // Prepare signal handling before spawning child diff --git a/alacritty_terminal/src/tty/windows/conpty.rs b/alacritty_terminal/src/tty/windows/conpty.rs index 44b3662f..28cdf4c4 100644 --- a/alacritty_terminal/src/tty/windows/conpty.rs +++ b/alacritty_terminal/src/tty/windows/conpty.rs @@ -18,10 +18,8 @@ use std::mem; use std::os::windows::io::IntoRawHandle; use std::ptr; -use dunce::canonicalize; use mio_anonymous_pipes::{EventedAnonRead, EventedAnonWrite}; use miow; -use widestring::U16CString; use winapi::shared::basetsd::{PSIZE_T, SIZE_T}; use winapi::shared::minwindef::{BYTE, DWORD}; use winapi::shared::ntdef::{HANDLE, HRESULT, LPWSTR}; @@ -34,11 +32,11 @@ use winapi::um::processthreadsapi::{ use winapi::um::winbase::{EXTENDED_STARTUPINFO_PRESENT, STARTF_USESTDHANDLES, STARTUPINFOEXW}; use winapi::um::wincontypes::{COORD, HPCON}; -use crate::config::{Config, Shell}; +use crate::config::Config; use crate::event::OnResize; use crate::term::SizeInfo; use crate::tty::windows::child::ChildExitWatcher; -use crate::tty::windows::Pty; +use crate::tty::windows::{cmdline, win32_string, Pty}; // TODO: Replace with winapi's implementation. This cannot be // done until a safety net is in place for versions of Windows @@ -141,8 +139,7 @@ pub fn new(config: &Config, size: &SizeInfo, _window_id: Option) -> let mut startup_info_ex: STARTUPINFOEXW = Default::default(); - let title = config.window.title.clone(); - let title = U16CString::from_str(title).unwrap(); + let title = win32_string(&config.window.title); startup_info_ex.StartupInfo.lpTitle = title.as_ptr() as LPWSTR; startup_info_ex.StartupInfo.cb = mem::size_of::() as u32; @@ -205,19 +202,11 @@ pub fn new(config: &Config, size: &SizeInfo, _window_id: Option) -> } } - // Get process commandline - let default_shell = &Shell::new("powershell"); - let shell = config.shell.as_ref().unwrap_or(default_shell); - let mut cmdline = shell.args.clone(); - cmdline.insert(0, shell.program.to_string()); - - // Warning, here be borrow hell - let cwd = config.working_directory().as_ref().map(|dir| canonicalize(dir).unwrap()); - let cwd = cwd.as_ref().map(|dir| dir.to_str().unwrap()); - - // Create the client application, using startup info containing ConPTY info - let cmdline = U16CString::from_str(&cmdline.join(" ")).unwrap(); - let cwd = cwd.map(|s| U16CString::from_str(&s).unwrap()); + let cmdline = win32_string(&cmdline(&config)); + let cwd = config + .working_directory() + .map(|dir| dir.canonicalize().unwrap()) + .map(|path| win32_string(&path)); let mut proc_info: PROCESS_INFORMATION = Default::default(); unsafe { diff --git a/alacritty_terminal/src/tty/windows/mod.rs b/alacritty_terminal/src/tty/windows/mod.rs index f5e0e61f..03e2a3cd 100644 --- a/alacritty_terminal/src/tty/windows/mod.rs +++ b/alacritty_terminal/src/tty/windows/mod.rs @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::ffi::OsStr; use std::io::{self, Read, Write}; +use std::iter::once; +use std::os::windows::ffi::OsStrExt; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::TryRecvError; @@ -22,7 +25,7 @@ use mio_named_pipes::NamedPipe; use log::info; -use crate::config::Config; +use crate::config::{Config, Shell}; use crate::event::OnResize; use crate::term::SizeInfo; use crate::tty::windows::child::ChildExitWatcher; @@ -298,3 +301,19 @@ impl OnResize for Pty { } } } + +fn cmdline(config: &Config) -> String { + let default_shell = Shell::new("powershell"); + let shell = config.shell.as_ref().unwrap_or(&default_shell); + + once(shell.program.as_ref()) + .chain(shell.args.iter().map(|a| a.as_ref())) + .collect::>() + .join(" ") +} + +/// Converts the string slice into a Windows-standard representation for "W"- +/// suffixed function variants, which accept UTF-16 encoded string values. +pub fn win32_string + ?Sized>(value: &S) -> Vec { + OsStr::new(value).encode_wide().chain(once(0)).collect() +} diff --git a/alacritty_terminal/src/tty/windows/winpty.rs b/alacritty_terminal/src/tty/windows/winpty.rs index 0d2421b0..c56ad8f6 100644 --- a/alacritty_terminal/src/tty/windows/winpty.rs +++ b/alacritty_terminal/src/tty/windows/winpty.rs @@ -17,17 +17,16 @@ use std::os::windows::fs::OpenOptionsExt; use std::os::windows::io::{FromRawHandle, IntoRawHandle}; use std::u16; -use dunce::canonicalize; use log::info; use mio_named_pipes::NamedPipe; use winapi::um::winbase::FILE_FLAG_OVERLAPPED; use winpty::{Config as WinptyConfig, ConfigFlags, MouseMode, SpawnConfig, SpawnFlags, Winpty}; -use crate::config::{Config, Shell}; +use crate::config::Config; use crate::event::OnResize; use crate::term::SizeInfo; use crate::tty::windows::child::ChildExitWatcher; -use crate::tty::windows::Pty; +use crate::tty::windows::{cmdline, Pty}; pub use winpty::Winpty as Agent; @@ -42,22 +41,15 @@ pub fn new(config: &Config, size: &SizeInfo, _window_id: Option) -> let mut agent = Winpty::open(&wconfig).unwrap(); let (conin, conout) = (agent.conin_name(), agent.conout_name()); - // Get process commandline - let default_shell = &Shell::new("powershell"); - let shell = config.shell.as_ref().unwrap_or(default_shell); - let mut cmdline = shell.args.clone(); - cmdline.insert(0, shell.program.to_string()); - - // Warning, here be borrow hell - let cwd = config.working_directory().as_ref().map(|dir| canonicalize(dir).unwrap()); - let cwd = cwd.as_ref().map(|dir| dir.to_str().unwrap()); + let cmdline = cmdline(&config); + let cwd = config.working_directory().map(|dir| dir.canonicalize().unwrap()); // Spawn process let spawnconfig = SpawnConfig::new( SpawnFlags::AUTO_SHUTDOWN | SpawnFlags::EXIT_AFTER_SHUTDOWN, None, // appname - Some(&cmdline.join(" ")), - cwd, + Some(&cmdline), + cwd.as_ref().map(|p| p.as_ref()), None, // Env ) .unwrap(); diff --git a/winpty/Cargo.toml b/winpty/Cargo.toml index 9d1b2d27..02cc21c7 100644 --- a/winpty/Cargo.toml +++ b/winpty/Cargo.toml @@ -9,7 +9,6 @@ edition = "2018" [target.'cfg(windows)'.dependencies] winpty-sys = "0.4.3" bitflags = "1.0" -widestring = "0.4.0" [target.'cfg(windows)'.dev-dependencies] named_pipe = "0.4.1" diff --git a/winpty/src/lib.rs b/winpty/src/lib.rs index 117ec16d..f3b7aff9 100644 --- a/winpty/src/lib.rs +++ b/winpty/src/lib.rs @@ -1,13 +1,5 @@ #![deny(clippy::all, clippy::if_not_else, clippy::enum_glob_use, clippy::wrong_pub_self_convention)] -#[macro_use] -#[cfg(windows)] -extern crate bitflags; -#[cfg(windows)] -extern crate widestring; -#[cfg(windows)] -extern crate winpty_sys; - #[cfg(windows)] pub mod windows; diff --git a/winpty/src/windows.rs b/winpty/src/windows.rs index 20cb3dc3..25bf9d3e 100644 --- a/winpty/src/windows.rs +++ b/winpty/src/windows.rs @@ -1,12 +1,15 @@ +use std::ffi::{OsStr, OsString}; use std::fmt::{self, Display, Formatter}; +use std::iter::once; +use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::os::windows::io::RawHandle; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::ptr::{null, null_mut}; use std::result::Result; -use winpty_sys::*; +use bitflags::bitflags; -use widestring::WideCString; +use winpty_sys::*; #[derive(Copy, Clone, Debug)] pub enum ErrorCode { @@ -167,7 +170,7 @@ impl Winpty { pub fn conin_name(&mut self) -> PathBuf { unsafe { let raw = winpty_conin_name(self.0); - PathBuf::from(&String::from_utf16_lossy(std::slice::from_raw_parts(raw, wcslen(raw)))) + OsString::from_wide(std::slice::from_raw_parts(raw, wcslen(raw))).into() } } @@ -176,7 +179,7 @@ impl Winpty { pub fn conout_name(&mut self) -> PathBuf { unsafe { let raw = winpty_conout_name(self.0); - PathBuf::from(&String::from_utf16_lossy(std::slice::from_raw_parts(raw, wcslen(raw)))) + OsString::from_wide(std::slice::from_raw_parts(raw, wcslen(raw))).into() } } @@ -186,7 +189,7 @@ impl Winpty { pub fn conerr_name(&mut self) -> PathBuf { unsafe { let raw = winpty_conerr_name(self.0); - PathBuf::from(&String::from_utf16_lossy(std::slice::from_raw_parts(raw, wcslen(raw)))) + OsString::from_wide(std::slice::from_raw_parts(raw, wcslen(raw))).into() } } @@ -283,25 +286,28 @@ impl SpawnConfig { spawnflags: SpawnFlags, appname: Option<&str>, cmdline: Option<&str>, - cwd: Option<&str>, - end: Option<&str>, + cwd: Option<&Path>, + env: Option<&str>, ) -> Result { let mut err = null_mut() as *mut winpty_error_t; - let to_wstring = |s| WideCString::from_str(s).unwrap(); + fn to_wstring + ?Sized>(s: &S) -> Vec { + OsStr::new(s).encode_wide().chain(once(0)).collect() + } + let appname = appname.map(to_wstring); let cmdline = cmdline.map(to_wstring); let cwd = cwd.map(to_wstring); - let end = end.map(to_wstring); + let env = env.map(to_wstring); - let wstring_ptr = |opt: &Option| opt.as_ref().map_or(null(), |ws| ws.as_ptr()); + let wstring_ptr = |opt: &Option>| opt.as_ref().map_or(null(), |ws| ws.as_ptr()); let spawn_config = unsafe { winpty_spawn_config_new( spawnflags.bits(), wstring_ptr(&appname), wstring_ptr(&cmdline), wstring_ptr(&cwd), - wstring_ptr(&end), + wstring_ptr(&env), &mut err, ) };