From d32fee74f9e6b88dbc0454b2e5c5b4f2d1c61e32 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 10:43:48 -0500 Subject: [PATCH 01/15] Add Projection type This can be used when you primarily want to return a reference but in order for that reference to live long enough it must be returned with an object. i.e. given `Mutex` you want a function to lock the mutex and return a reference to `bar` but you can't return that reference since it has a lifetime dependency on `MutexGuard` (which only derefs to all of `Foo` and not just `bar`). You can return a `Projection` owning the `MutexGuard` and set it up to deref to `&bar`. --- fish-rust/src/common.rs | 50 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/fish-rust/src/common.rs b/fish-rust/src/common.rs index cb154f08b..9b29cdd3c 100644 --- a/fish-rust/src/common.rs +++ b/fish-rust/src/common.rs @@ -1985,6 +1985,56 @@ pub fn get_by_sorted_name(name: &wstr, vals: &'static [T]) -> Option<& } } +/// Takes ownership of a variable and `Deref`s/`DerefMut`s into a projection of that variable. +/// +/// Can be used as a workaround for the lack of `MutexGuard::map()` to return a `MutexGuard` +/// exposing only a variable of the Mutex-owned object. +pub struct Projection +where + F1: Fn(&T) -> &V, + F2: Fn(&mut T) -> &mut V, +{ + value: T, + view: F1, + view_mut: F2, +} + +impl Projection +where + F1: Fn(&T) -> &V, + F2: Fn(&mut T) -> &mut V, +{ + pub fn new(owned: T, project: F1, project_mut: F2) -> Self { + Projection { + value: owned, + view: project, + view_mut: project_mut, + } + } +} + +impl Deref for Projection +where + F1: Fn(&T) -> &V, + F2: Fn(&mut T) -> &mut V, +{ + type Target = V; + + fn deref(&self) -> &Self::Target { + (self.view)(&self.value) + } +} + +impl DerefMut for Projection +where + F1: Fn(&T) -> &V, + F2: Fn(&mut T) -> &mut V, +{ + fn deref_mut(&mut self) -> &mut Self::Target { + (self.view_mut)(&mut self.value) + } +} + #[allow(unused_macros)] macro_rules! fwprintf { ($fd:expr, $format:literal $(, $arg:expr)*) => { From 6fc89400976cc2caa9a114a8119a5f9c33187d33 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 11:58:29 -0500 Subject: [PATCH 02/15] Simplify ScopeGuard and scoped_push() with Projection Delegate the `view` and `view_mut` to the newly added `Projection`, which makes everything oh so much clearer and cleaner. Add comments to clarify what is happening. --- fish-rust/src/common.rs | 85 ++++++++++++----------------------------- 1 file changed, 24 insertions(+), 61 deletions(-) diff --git a/fish-rust/src/common.rs b/fish-rust/src/common.rs index 9b29cdd3c..205ed6f45 100644 --- a/fish-rust/src/common.rs +++ b/fish-rust/src/common.rs @@ -1715,7 +1715,7 @@ pub fn replace_with T>(old: &mut T, with: F) -> T { std::mem::replace(old, new) } -pub type Cleanup = ScopeGuard; +pub type Cleanup = ScopeGuard; /// A RAII cleanup object. Unlike in C++ where there is no borrow checker, we can't just provide a /// callback that modifies live objects willy-nilly because then there would be two &mut references @@ -1742,44 +1742,24 @@ pub type Cleanup = ScopeGuard; /// /// // hello will be written first, then goodbye. /// ``` -pub struct ScopeGuard { +pub struct ScopeGuard { captured: ManuallyDrop, - view: fn(&T) -> &C, - view_mut: fn(&mut T) -> &mut C, on_drop: Option, - marker: std::marker::PhantomData, } -fn identity(t: &T) -> &T { - t -} -fn identity_mut(t: &mut T) -> &mut T { - t -} - -impl ScopeGuard { +impl ScopeGuard +where + F: FnOnce(&mut T), +{ /// Creates a new `ScopeGuard` wrapping `value`. The `on_drop` callback is executed when the /// ScopeGuard's lifetime expires or when it is manually dropped. pub fn new(value: T, on_drop: F) -> Self { - Self::with_view(value, identity, identity_mut, on_drop) - } -} - -impl ScopeGuard { - pub fn with_view( - value: T, - view: fn(&T) -> &C, - view_mut: fn(&mut T) -> &mut C, - on_drop: F, - ) -> Self { Self { captured: ManuallyDrop::new(value), - view, - view_mut, on_drop: Some(on_drop), - marker: Default::default(), } } + /// Cancel the unwind operation, e.g. do not call the previously passed-in `on_drop` callback /// when the current scope expires. pub fn cancel(guard: &mut Self) { @@ -1807,21 +1787,21 @@ impl ScopeGuard { } } -impl Deref for ScopeGuard { - type Target = C; +impl Deref for ScopeGuard { + type Target = T; fn deref(&self) -> &Self::Target { - (self.view)(&self.captured) + &self.captured } } -impl DerefMut for ScopeGuard { +impl DerefMut for ScopeGuard { fn deref_mut(&mut self) -> &mut Self::Target { - (self.view_mut)(&mut self.captured) + &mut self.captured } } -impl Drop for ScopeGuard { +impl Drop for ScopeGuard { fn drop(&mut self) { if let Some(on_drop) = self.on_drop.take() { on_drop(&mut self.captured); @@ -1833,46 +1813,29 @@ impl Drop for ScopeGuard { /// A scoped manager to save the current value of some variable, and set it to a new value. When /// dropped, it restores the variable to its old value. -#[allow(clippy::type_complexity)] // Not sure how to extract the return type. pub fn scoped_push( mut ctx: Context, accessor: Accessor, new_value: T, -) -> ScopeGuard<(Context, Accessor, T), fn(&mut (Context, Accessor, T)), Context> +) -> impl Deref + DerefMut where Accessor: Fn(&mut Context) -> &mut T, T: Copy, { - fn restore_saved_value(data: &mut (Context, Accessor, T)) - where - Accessor: Fn(&mut Context) -> &mut T, - { - let (ref mut ctx, ref accessor, saved_value) = data; - *accessor(ctx) = *saved_value; - } - fn view_context(data: &(Context, Accessor, T)) -> &Context - where - Accessor: Fn(&mut Context) -> &mut T, - { - &data.0 - } - fn view_context_mut(data: &mut (Context, Accessor, T)) -> &mut Context - where - Accessor: Fn(&mut Context) -> &mut T, - { - &mut data.0 - } let saved_value = mem::replace(accessor(&mut ctx), new_value); - ScopeGuard::with_view( - (ctx, accessor, saved_value), - view_context, - view_context_mut, - restore_saved_value, - ) + // Store the original/root value, the function to map from the original value to the variables + // we are changing, and a saved snapshot of the previous values of those variables in a tuple, + // then use ScopeGuard's `on_drop` parameter to restore the saved values when the scope ends. + let scope_guard = ScopeGuard::new((ctx, accessor, saved_value), |data| { + let (ref mut ctx, accessor, saved_value) = data; + *accessor(ctx) = *saved_value; + }); + // `scope_guard` would deref to the tuple we gave it, so use Projection to map from the tuple + // `(ctx, accessor, saved_value)` to the result of `accessor(ctx)`. + Projection::new(scope_guard, |sg| &sg.0, |sg| &mut sg.0) } pub const fn assert_send() {} - pub const fn assert_sync() {} /// This function attempts to distinguish between a console session (at the actual login vty) and a From b17124d8d23fc9c438b49f5b78593d4fb93278fb Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 13:02:22 -0500 Subject: [PATCH 03/15] Add rsconf build system and check for gettext symbols This is more complicated than it needs to be thanks to the presence of CMake and the C++ ffi in the picture. rsconf can correctly detect the required libraries and instruct rustc to link against them, but since we generate a static rust library and have CMake link it against the C++ binaries, we are still at the mercy of CMake picking up the symbols we want. Unfortunately, we could detect the gettext symbols but discover at runtime that they weren't linked in because CMake was compiled with `-DWITH_GETTEXT=0` or similar (as the macOS CI runner does). This means we also need to pass state between CMake and our build script to communicate which CMake options were enabled. --- cmake/Rust.cmake | 9 +++++ fish-rust/Cargo.lock | 9 +++++ fish-rust/Cargo.toml | 1 + fish-rust/build.rs | 81 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/cmake/Rust.cmake b/cmake/Rust.cmake index bd836fed6..3ec5482e6 100644 --- a/cmake/Rust.cmake +++ b/cmake/Rust.cmake @@ -51,12 +51,21 @@ else() corrosion_set_hostbuild(${fish_rust_target}) endif() +# Temporary hack to propogate CMake flags/options to build.rs. We need to get CMake to evaluate the +# truthiness of the strings if they are set. +set(CMAKE_WITH_GETTEXT "1") +if(DEFINED WITH_GETTEXT AND NOT "${WITH_GETTEXT}") + set(CMAKE_WITH_GETTEXT "0") +endif() + # Tell Cargo where our build directory is so it can find config.h. corrosion_set_env_vars(${fish_rust_target} "FISH_BUILD_DIR=${CMAKE_BINARY_DIR}" "FISH_AUTOCXX_GEN_DIR=${fish_autocxx_gen_dir}" "FISH_RUST_TARGET_DIR=${rust_target_dir}" "PREFIX=${CMAKE_INSTALL_PREFIX}" + # Temporary hack to propogate CMake flags/options to build.rs. + "CMAKE_WITH_GETTEXT=${CMAKE_WITH_GETTEXT}" ) target_include_directories(${fish_rust_target} INTERFACE diff --git a/fish-rust/Cargo.lock b/fish-rust/Cargo.lock index 397b94178..6440fa0fa 100644 --- a/fish-rust/Cargo.lock +++ b/fish-rust/Cargo.lock @@ -356,6 +356,7 @@ dependencies = [ "pcre2", "printf-compat", "rand", + "rsconf", "unixstring", "widestring", "widestring-suffix", @@ -816,6 +817,14 @@ version = "0.6.29" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" +[[package]] +name = "rsconf" +version = "0.1.0" +source = "git+https://github.com/mqudsi/rsconf?branch=master#5966dd64796528e79e0dc9ba61b1dac679640273" +dependencies = [ + "cc", +] + [[package]] name = "rustc-hash" version = "1.1.0" diff --git a/fish-rust/Cargo.toml b/fish-rust/Cargo.toml index db042e9cd..317f6b174 100644 --- a/fish-rust/Cargo.toml +++ b/fish-rust/Cargo.toml @@ -32,6 +32,7 @@ autocxx-build = "0.23.1" cc = { git = "https://github.com/mqudsi/cc-rs", branch = "fish" } cxx-build = { git = "https://github.com/fish-shell/cxx", branch = "fish" } cxx-gen = { git = "https://github.com/fish-shell/cxx", branch = "fish" } +rsconf = { git = "https://github.com/mqudsi/rsconf", branch = "master" } [lib] crate-type = ["staticlib"] diff --git a/fish-rust/build.rs b/fish-rust/build.rs index eeb809753..cc661871a 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -1,3 +1,4 @@ +use rsconf::{LinkType, Target}; use std::error::Error; fn main() { @@ -19,7 +20,19 @@ fn main() { let autocxx_gen_dir = std::env::var("FISH_AUTOCXX_GEN_DIR") .unwrap_or(format!("{}/{}", fish_build_dir, "fish-autocxx-gen/")); - detect_features(); + let mut build = cc::Build::new(); + // Add to the default library search path + build.flag_if_supported("-L/usr/local/lib/"); + rsconf::add_library_search_path("/usr/local/lib"); + let mut detector = Target::new_from(build).unwrap(); + // Keep verbose mode on until we've ironed out rust build script stuff + // Note that if autocxx fails to compile any rust code, you'll see the full and unredacted + // stdout/stderr output, which will include things that LOOK LIKE compilation errors as rsconf + // tries to build various test files to try and figure out which libraries and symbols are + // available. IGNORE THESE and scroll to the very bottom of the build script output, past all + // these errors, to see the actual issue. + detector.set_verbose(true); + detect_features(detector); // Emit cxx junk. // This allows "Rust to be used from C++" @@ -80,9 +93,7 @@ fn main() { b.flag_if_supported("-std=c++11") .flag("-Wno-comment") .compile("fish-rust-autocxx"); - for file in source_files { - println!("cargo:rerun-if-changed={file}"); - } + rsconf::rebuild_if_paths_changed(&source_files); } /// Dynamically enables certain features at build-time, without their having to be explicitly @@ -93,19 +104,20 @@ fn main() { /// `Cargo.toml`) behind a feature we just enabled. /// /// [0]: https://github.com/rust-lang/cargo/issues/5499 -fn detect_features() { - for (feature, detector) in [ - // Ignore the first line, it just sets up the type inference. Model new entries after the +fn detect_features(target: Target) { + for (feature, handler) in [ + // Ignore the first entry, it just sets up the type inference. Model new entries after the // second line. ( "", - &(|| Ok(false)) as &dyn Fn() -> Result>, + &(|_: &Target| Ok(false)) as &dyn Fn(&Target) -> Result>, ), ("bsd", &detect_bsd), + ("gettext", &have_gettext), ] { - match detector() { - Err(e) => eprintln!("ERROR: {feature} detect: {e}"), - Ok(true) => println!("cargo:rustc-cfg=feature=\"{feature}\""), + match handler(&target) { + Err(e) => rsconf::warn!("{}: {}", feature, e), + Ok(true) => rsconf::enable_feature(feature), Ok(false) => (), } } @@ -117,7 +129,7 @@ fn detect_features() { /// Rust offers fine-grained conditional compilation per-os for the popular operating systems, but /// doesn't necessarily include less-popular forks nor does it group them into families more /// specific than "windows" vs "unix" so we can conditionally compile code for BSD systems. -fn detect_bsd() -> Result> { +fn detect_bsd(_: &Target) -> Result> { // Instead of using `uname`, we can inspect the TARGET env variable set by Cargo. This lets us // support cross-compilation scenarios. let mut target = std::env::var("TARGET").unwrap(); @@ -134,3 +146,48 @@ fn detect_bsd() -> Result> { assert!(result, "Target incorrectly detected as not BSD!"); Ok(result) } + +/// Detect libintl/gettext and its needed symbols to enable internationalization/localization +/// support. +fn have_gettext(target: &Target) -> Result> { + // The following script correctly detects and links against gettext, but so long as we are using + // C++ and generate a static library linked into the C++ binary via CMake, we need to account + // for the CMake option WITH_GETTEXT being explicitly disabled. + rsconf::rebuild_if_env_changed("CMAKE_WITH_GETTEXT"); + if let Some(with_gettext) = std::env::var_os("CMAKE_WITH_GETTEXT") { + if with_gettext.eq_ignore_ascii_case("0") { + return Ok(false); + } + } + + // In order for fish to correctly operate, we need some way of notifying libintl to invalidate + // its localizations when the locale environment variables are modified. Without the libintl + // symbol _nl_msg_cat_cntr, we cannot use gettext even if we find it. + let mut libraries = Vec::new(); + let mut found = 0; + let symbols = ["gettext", "_nl_msg_cat_cntr"]; + for symbol in &symbols { + // Historically, libintl was required in order to use gettext() and co, but that + // functionality was subsumed by some versions of libc. + if target.has_symbol_in::<&str>(symbol, &[]) { + // No need to link anything special for this symbol + found += 1; + continue; + } + for library in ["intl", "gettextlib"] { + if target.has_symbol(symbol, library) { + libraries.push(library); + found += 1; + continue; + } + } + } + match found { + 0 => Ok(false), + 1 => Err(format!("gettext found but cannot be used without {}", symbols[1]).into()), + _ => { + rsconf::link_libraries(&libraries, LinkType::Default); + Ok(true) + } + } +} From 77dda2cdef7ee8c786582fbbca20ae4728d365dc Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 13:10:31 -0500 Subject: [PATCH 04/15] Add ToCString trait This can be used for functions that accept non-Unicode content (i.e. &CStr or CString) but are often used in our code base with a UTF-8 or UTF-32 string on-hand. When such a function is passed a CString, it's passed through as-is and allocation-free. But when, as is often the case, we have a static string we can now pass it in directly with all the nice ergonomics thereof instead of having to manually create and unwrap a CString at the call location. There's an upstream request to add this functionality to the standard library: https://github.com/rust-lang/rust/issues/71448 --- fish-rust/src/common.rs | 62 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/fish-rust/src/common.rs b/fish-rust/src/common.rs index 205ed6f45..51e9dac8e 100644 --- a/fish-rust/src/common.rs +++ b/fish-rust/src/common.rs @@ -23,7 +23,7 @@ use libc::{EINTR, EIO, O_WRONLY, SIGTTOU, SIG_IGN, STDERR_FILENO, STDIN_FILENO, use num_traits::ToPrimitive; use once_cell::sync::Lazy; use std::env; -use std::ffi::{CString, OsString}; +use std::ffi::{CStr, CString, OsString}; use std::mem::{self, ManuallyDrop}; use std::ops::{Deref, DerefMut}; use std::os::fd::{AsRawFd, RawFd}; @@ -34,6 +34,7 @@ use std::str::FromStr; use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; use std::sync::Mutex; use std::time; +use widestring::Utf32String; use widestring_suffix::widestrs; // Highest legal ASCII value. @@ -1998,6 +1999,65 @@ where } } +/// A trait to make it more convenient to pass ascii/Unicode strings to functions that can take +/// non-Unicode values. The result is nul-terminated and can be passed to OS functions. +/// +/// This is only implemented for owned types where an owned instance will skip allocations (e.g. +/// `CString` can return `self`) but not implemented for owned instances where a new allocation is +/// always required (e.g. implemented for `&wstr` but not `WideString`) because you might as well be +/// left with the original item if we're going to allocate from scratch in all cases. +pub trait ToCString { + /// Correctly convert to a nul-terminated [`CString`] that can be passed to OS functions. + fn to_cstring(self) -> CString; +} + +impl ToCString for CString { + fn to_cstring(self) -> CString { + self + } +} + +impl ToCString for &CStr { + fn to_cstring(self) -> CString { + self.to_owned() + } +} + +/// Safely converts from `&wstr` to a `CString` to a nul-terminated `CString` that can be passed to +/// OS functions, taking into account non-Unicode values that have been shifted into the private-use +/// range by using [`wcs2zstring()`]. +impl ToCString for &wstr { + /// The wide string may contain non-Unicode bytes mapped to the private-use Unicode range, so we + /// have to use [`wcs2zstring()`](self::wcs2zstring) to convert it correctly. + fn to_cstring(self) -> CString { + self::wcs2zstring(self) + } +} + +/// Safely converts from `&Utf32String` to a nul-terminated `CString` that can be passed to OS +/// functions, taking into account non-Unicode values that have been shifted into the private-use +/// range by using [`wcs2zstring()`]. +impl ToCString for &Utf32String { + fn to_cstring(self) -> CString { + self.as_utfstr().to_cstring() + } +} + +/// Convert a (probably ascii) string to CString that can be passed to OS functions. +impl ToCString for Vec { + fn to_cstring(mut self) -> CString { + self.push(b'\0'); + CString::from_vec_with_nul(self).unwrap() + } +} + +/// Convert a (probably ascii) string to nul-terminated CString that can be passed to OS functions. +impl ToCString for &[u8] { + fn to_cstring(self) -> CString { + CString::new(self).unwrap() + } +} + #[allow(unused_macros)] macro_rules! fwprintf { ($fd:expr, $format:literal $(, $arg:expr)*) => { From e154391f3271868eecdd8860ff4949cca3b9d889 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 13:17:34 -0500 Subject: [PATCH 05/15] Add WCharExt::find() method to perform substring search --- fish-rust/src/wchar_ext.rs | 52 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/fish-rust/src/wchar_ext.rs b/fish-rust/src/wchar_ext.rs index c6715f10a..bb568474a 100644 --- a/fish-rust/src/wchar_ext.rs +++ b/fish-rust/src/wchar_ext.rs @@ -226,6 +226,15 @@ pub trait WExt { } } + /// Returns the index of the first match against the provided substring or `None`. + fn find(&self, search: impl AsRef<[char]>) -> Option { + fn inner(lhs: &[char], rhs: &[char]) -> Option { + lhs.windows(rhs.len()).position(|window| window == rhs) + } + + inner(self.as_char_slice(), search.as_ref()) + } + /// \return the index of the first occurrence of the given char, or None. fn find_char(&self, c: char) -> Option { self.as_char_slice().iter().position(|&x| x == c) @@ -318,4 +327,47 @@ mod tests { &["Hello", "world", "Rust"] ); } + + #[test] + fn find_prefix() { + let needle = L!("hello"); + let haystack = L!("hello world"); + assert_eq!(haystack.find(needle), Some(0)); + } + + #[test] + fn find_one() { + let needle = L!("ello"); + let haystack = L!("hello world"); + assert_eq!(haystack.find(needle), Some(1)); + } + + #[test] + fn find_suffix() { + let needle = L!("world"); + let haystack = L!("hello world"); + assert_eq!(haystack.find(needle), Some(6)); + } + + #[test] + fn find_none() { + let needle = L!("worldz"); + let haystack = L!("hello world"); + assert_eq!(haystack.find(needle), None); + } + + #[test] + fn find_none_larger() { + // Notice that `haystack` and `needle` are reversed. + let haystack = L!("world"); + let needle = L!("hello world"); + assert_eq!(haystack.find(needle), None); + } + + #[test] + fn find_none_case_mismatch() { + let haystack = L!("wOrld"); + let needle = L!("hello world"); + assert_eq!(haystack.find(needle), None); + } } From 1a88c55b71fe010f7fdb823b5dda58aabe6be8ee Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 13:55:38 -0500 Subject: [PATCH 06/15] Clean up FISH_EMOJI_WIDTH and FISH_AMBIGUOUS_WIDTH defines Pull in the correct descriptions merged from across the various C++ header and source files and get rid of the getter function that's only used in one place but causes us to split the documentation for FISH_EMOJI_WIDTH across multiple declarations. --- fish-rust/src/fallback.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/fish-rust/src/fallback.rs b/fish-rust/src/fallback.rs index d429d0c2b..5c3a575ff 100644 --- a/fish-rust/src/fallback.rs +++ b/fish-rust/src/fallback.rs @@ -10,16 +10,21 @@ use std::cmp; use std::sync::atomic::{AtomicI32, Ordering}; use std::{ffi::CString, mem, os::fd::RawFd}; -// Width of ambiguous characters. 1 is typical default. -static FISH_AMBIGUOUS_WIDTH: AtomicI32 = AtomicI32::new(1); +/// Width of ambiguous East Asian characters and, as of TR11, all private-use characters. +/// 1 is the typical default, but we accept any non-negative override via `$fish_ambiguous_width`. +pub static FISH_AMBIGUOUS_WIDTH: AtomicI32 = AtomicI32::new(1); -// Width of emoji characters. -// 1 is the typical emoji width in Unicode 8. -static FISH_EMOJI_WIDTH: AtomicI32 = AtomicI32::new(1); - -fn fish_get_emoji_width() -> i32 { - FISH_EMOJI_WIDTH.load(Ordering::Relaxed) -} +/// Width of emoji characters. +/// +/// This must be configurable because the value changed between Unicode 8 and Unicode 9, `wcwidth()` +/// is emoji-unaware, and terminal emulators do different things. +/// +/// See issues like #4539 and https://github.com/neovim/issues/4976 for how painful this is. +/// +/// Valid values are 1, and 2. 1 is the typical emoji width used in Unicode 8 while some newer +/// terminals use a width of 2 since Unicode 9. +// For some reason, this is declared here and exposed here, but is set in `env_dispatch`. +pub static FISH_EMOJI_WIDTH: AtomicI32 = AtomicI32::new(1); extern "C" { pub fn wcwidth(c: libc::wchar_t) -> libc::c_int; @@ -71,7 +76,7 @@ pub fn fish_wcwidth(c: char) -> i32 { } WcWidth::One => 1, WcWidth::Two => 2, - WcWidth::WidenedIn9 => fish_get_emoji_width(), + WcWidth::WidenedIn9 => FISH_EMOJI_WIDTH.load(Ordering::Relaxed), } } From 3ee71772f11248d4545a348fb01b688e6c7a8ba3 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 18:54:10 -0500 Subject: [PATCH 07/15] Revert rename of wcwidth() to system_wcwidth() It's not clear whether or not `system_wcwidth()` was picked solely because of the namespace conflict (which is easily remedied) but using the most obvious name for this function should be the way to go. We already have our own overload of `wcwidth()` (`fish_wcwidth()`) so it should be more obvious which is the bare system call and which isn't. (I do want to move this w/ some of the other standalone extern C wrappers to the unix module later.) --- fish-rust/src/fallback.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/fish-rust/src/fallback.rs b/fish-rust/src/fallback.rs index 5c3a575ff..f0ff63a31 100644 --- a/fish-rust/src/fallback.rs +++ b/fish-rust/src/fallback.rs @@ -26,16 +26,18 @@ pub static FISH_AMBIGUOUS_WIDTH: AtomicI32 = AtomicI32::new(1); // For some reason, this is declared here and exposed here, but is set in `env_dispatch`. pub static FISH_EMOJI_WIDTH: AtomicI32 = AtomicI32::new(1); -extern "C" { - pub fn wcwidth(c: libc::wchar_t) -> libc::c_int; -} -fn system_wcwidth(c: char) -> i32 { +static WC_LOOKUP_TABLE: Lazy = Lazy::new(WcLookupTable::new); + +/// A safe wrapper around the system `wcwidth()` function +pub fn wcwidth(c: char) -> i32 { + extern "C" { + pub fn wcwidth(c: libc::wchar_t) -> libc::c_int; + } + const _: () = assert!(mem::size_of::() >= mem::size_of::()); unsafe { wcwidth(c as libc::wchar_t) } } -static WC_LOOKUP_TABLE: Lazy = Lazy::new(WcLookupTable::new); - // Big hack to use our versions of wcswidth where we know them to be broken, which is // EVERYWHERE (https://github.com/fish-shell/fish-shell/issues/2199) pub fn fish_wcwidth(c: char) -> i32 { @@ -43,7 +45,7 @@ pub fn fish_wcwidth(c: char) -> i32 { // in the console session, but knows nothing about the capabilities of other terminal emulators // or ttys. Use it from the start only if we are logged in to the physical console. if is_console_session() { - return system_wcwidth(c); + return wcwidth(c); } // Check for VS16 which selects emoji presentation. This "promotes" a character like U+2764 @@ -68,7 +70,7 @@ pub fn fish_wcwidth(c: char) -> i32 { match width { WcWidth::NonCharacter | WcWidth::NonPrint | WcWidth::Combining | WcWidth::Unassigned => { // Fall back to system wcwidth in this case. - system_wcwidth(c) + wcwidth(c) } WcWidth::Ambiguous | WcWidth::PrivateUse => { // TR11: "All private-use characters are by default classified as Ambiguous". From 3ab8b34b1e9ff0bd15eb7afc9071dd043971664d Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 14:09:30 -0500 Subject: [PATCH 08/15] Use Rust version of global fallback variables --- fish-rust/src/fallback.rs | 2 ++ src/env_dispatch.cpp | 16 ++++++++-------- src/fallback.cpp | 11 ++--------- src/fallback.h | 14 +++++++------- 4 files changed, 19 insertions(+), 24 deletions(-) diff --git a/fish-rust/src/fallback.rs b/fish-rust/src/fallback.rs index f0ff63a31..01ddd900f 100644 --- a/fish-rust/src/fallback.rs +++ b/fish-rust/src/fallback.rs @@ -12,6 +12,7 @@ use std::{ffi::CString, mem, os::fd::RawFd}; /// Width of ambiguous East Asian characters and, as of TR11, all private-use characters. /// 1 is the typical default, but we accept any non-negative override via `$fish_ambiguous_width`. +#[no_mangle] pub static FISH_AMBIGUOUS_WIDTH: AtomicI32 = AtomicI32::new(1); /// Width of emoji characters. @@ -24,6 +25,7 @@ pub static FISH_AMBIGUOUS_WIDTH: AtomicI32 = AtomicI32::new(1); /// Valid values are 1, and 2. 1 is the typical emoji width used in Unicode 8 while some newer /// terminals use a width of 2 since Unicode 9. // For some reason, this is declared here and exposed here, but is set in `env_dispatch`. +#[no_mangle] pub static FISH_EMOJI_WIDTH: AtomicI32 = AtomicI32::new(1); static WC_LOOKUP_TABLE: Lazy = Lazy::new(WcLookupTable::new); diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index 8ff4adfdb..58b3d3a23 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -149,13 +149,13 @@ static void handle_timezone(const wchar_t *env_var_name, const environment_t &va tzset(); } -/// Update the value of g_fish_emoji_width +/// Update the value of FISH_EMOJI_WIDTH static void guess_emoji_width(const environment_t &vars) { if (auto width_str = vars.get(L"fish_emoji_width")) { int new_width = fish_wcstol(width_str->as_string().c_str()); - g_fish_emoji_width = std::min(2, std::max(1, new_width)); + FISH_EMOJI_WIDTH = std::min(2, std::max(1, new_width)); FLOGF(term_support, "'fish_emoji_width' preference: %d, overwriting default", - g_fish_emoji_width); + FISH_EMOJI_WIDTH); return; } @@ -172,18 +172,18 @@ static void guess_emoji_width(const environment_t &vars) { if (term == L"Apple_Terminal" && version >= 400) { // Apple Terminal on High Sierra - g_fish_emoji_width = 2; + FISH_EMOJI_WIDTH = 2; FLOGF(term_support, "default emoji width: 2 for %ls", term.c_str()); } else if (term == L"iTerm.app") { // iTerm2 now defaults to Unicode 9 sizes for anything after macOS 10.12. - g_fish_emoji_width = 2; + FISH_EMOJI_WIDTH = 2; FLOGF(term_support, "default emoji width for iTerm: 2"); } else { // Default to whatever system wcwidth says to U+1F603, // but only if it's at least 1 and at most 2. int w = wcwidth(L'😃'); - g_fish_emoji_width = std::min(2, std::max(1, w)); - FLOGF(term_support, "default emoji width: %d", g_fish_emoji_width); + FISH_EMOJI_WIDTH = std::min(2, std::max(1, w)); + FLOGF(term_support, "default emoji width: %d", FISH_EMOJI_WIDTH); } } @@ -209,7 +209,7 @@ static void handle_change_ambiguous_width(const env_stack_t &vars) { if (auto width_str = vars.get(L"fish_ambiguous_width")) { new_width = fish_wcstol(width_str->as_string().c_str()); } - g_fish_ambiguous_width = std::max(0, new_width); + FISH_AMBIGUOUS_WIDTH = std::max(0, new_width); } static void handle_term_size_change(const env_stack_t &vars) { diff --git a/src/fallback.cpp b/src/fallback.cpp index 966bb28aa..e022f3cab 100644 --- a/src/fallback.cpp +++ b/src/fallback.cpp @@ -129,16 +129,9 @@ int killpg(int pgr, int sig) { } #endif -// Width of ambiguous characters. 1 is typical default. -int g_fish_ambiguous_width = 1; - -// Width of emoji characters. -// 1 is the typical emoji width in Unicode 8. -int g_fish_emoji_width = 1; - static int fish_get_emoji_width(wchar_t c) { (void)c; - return g_fish_emoji_width; + return FISH_EMOJI_WIDTH; } // Big hack to use our versions of wcswidth where we know them to be broken, which is @@ -179,7 +172,7 @@ int fish_wcwidth(wchar_t wc) { case widechar_ambiguous: case widechar_private_use: // TR11: "All private-use characters are by default classified as Ambiguous". - return g_fish_ambiguous_width; + return FISH_AMBIGUOUS_WIDTH; case widechar_widened_in_9: return fish_get_emoji_width(wc); default: diff --git a/src/fallback.h b/src/fallback.h index 79ed82812..b56caafe1 100644 --- a/src/fallback.h +++ b/src/fallback.h @@ -1,6 +1,7 @@ #ifndef FISH_FALLBACK_H #define FISH_FALLBACK_H +#include #include "config.h" // The following include must be kept despite what IWYU says. That's because of the interaction @@ -8,15 +9,14 @@ // in . At least on OS X if we don't do this we get compilation errors do to the macro // substitution if wchar.h is included after this header. #include // IWYU pragma: keep + // +// Width of ambiguous characters. 1 is typical default. +extern int32_t FISH_AMBIGUOUS_WIDTH; -/// The column width of ambiguous East Asian characters. -extern int g_fish_ambiguous_width; +// Width of emoji characters. +// 1 is the typical emoji width in Unicode 8. +extern int32_t FISH_EMOJI_WIDTH; -/// The column width of emoji characters. This must be configurable because the value changed -/// between Unicode 8 and Unicode 9, wcwidth() is emoji-ignorant, and terminal emulators do -/// different things. See issues like #4539 and https://github.com/neovim/neovim/issues/4976 for how -/// painful this is. A value of 0 means to use the guessed value. -extern int g_fish_emoji_width; /// fish's internal versions of wcwidth and wcswidth, which can use an internal implementation if /// the system one is busted. From 8a549cbb15e693a61483268ae25fdf9e3c8f15ff Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 14:35:14 -0500 Subject: [PATCH 09/15] Port/move some code from src/environment.cpp to src/env/mod.rs The global variables are moved (not copied) from C++ to rust and exported as extern C integers. On the rust side they are accessed only with atomic semantics but regular int access is preserved from the C++ side (until that code is also ported). --- fish-rust/src/env/mod.rs | 52 ++++++++++++++++++++++++++++++++++++++ src/builtins/read.cpp | 4 +-- src/builtins/set_color.cpp | 2 +- src/env.cpp | 7 ++--- src/env.h | 14 +++++++--- src/env_dispatch.cpp | 9 +++---- src/exec.cpp | 2 +- src/input.cpp | 2 +- src/screen.cpp | 2 +- 9 files changed, 74 insertions(+), 20 deletions(-) diff --git a/fish-rust/src/env/mod.rs b/fish-rust/src/env/mod.rs index f6787bff4..f901bad9e 100644 --- a/fish-rust/src/env/mod.rs +++ b/fish-rust/src/env/mod.rs @@ -3,6 +3,58 @@ pub mod environment; mod environment_impl; pub mod var; +use crate::common::ToCString; pub use env_ffi::EnvStackSetResult; pub use environment::*; +use std::sync::atomic::{AtomicBool, AtomicUsize}; pub use var::*; + +/// Limit `read` to 100 MiB (bytes, not wide chars) by default. This can be overriden with the +/// `fish_read_limit` variable. +pub const DEFAULT_READ_BYTE_LIMIT: usize = 100 * 1024 * 1024; + +/// The actual `read` limit in effect, defaulting to [`DEFAULT_READ_BYTE_LIMIT`] but overridable +/// with `$fish_read_limit`. +#[no_mangle] +pub static READ_BYTE_LIMIT: AtomicUsize = AtomicUsize::new(DEFAULT_READ_BYTE_LIMIT); + +/// The curses `cur_term` TERMINAL pointer has been set up. +#[no_mangle] +pub static CURSES_INITIALIZED: AtomicBool = AtomicBool::new(false); + +/// Does the terminal have the "eat new line" glitch. +#[no_mangle] +pub static TERM_HAS_XN: AtomicBool = AtomicBool::new(false); + +mod ffi { + extern "C" { + pub fn setenv_lock( + name: *const libc::c_char, + value: *const libc::c_char, + overwrite: libc::c_int, + ); + pub fn unsetenv_lock(name: *const libc::c_char); + } +} + +/// Sets an environment variable after obtaining a lock, to try and improve the safety of +/// environment variables. +/// +/// As values could contain non-unicode characters, they must first be converted from &wstr to a +/// `CString` with [`crate::common::wcs2zstring()`]. +pub fn setenv_lock(name: S1, value: S2, overwrite: bool) { + let name = name.to_cstring(); + let value = value.to_cstring(); + unsafe { + self::ffi::setenv_lock(name.as_ptr(), value.as_ptr(), libc::c_int::from(overwrite)); + } +} + +/// Unsets an environment variable after obtaining a lock, to try and improve the safety of +/// environment variables. +pub fn unsetenv_lock(name: S) { + unsafe { + let name = name.to_cstring(); + self::ffi::unsetenv_lock(name.as_ptr()); + } +} diff --git a/src/builtins/read.cpp b/src/builtins/read.cpp index 8682f8658..50172a9d0 100644 --- a/src/builtins/read.cpp +++ b/src/builtins/read.cpp @@ -281,7 +281,7 @@ static int read_in_chunks(int fd, wcstring &buff, bool split_null, bool do_seek) return STATUS_CMD_ERROR; } finished = true; - } else if (str.size() > read_byte_limit) { + } else if (str.size() > READ_BYTE_LIMIT) { exit_res = STATUS_READ_TOO_MUCH; finished = true; } @@ -329,7 +329,7 @@ static int read_one_char_at_a_time(int fd, wcstring &buff, int nchars, bool spli } } - if (nbytes > read_byte_limit) { + if (nbytes > READ_BYTE_LIMIT) { exit_res = STATUS_READ_TOO_MUCH; break; } diff --git a/src/builtins/set_color.cpp b/src/builtins/set_color.cpp index 4299832ae..a42802174 100644 --- a/src/builtins/set_color.cpp +++ b/src/builtins/set_color.cpp @@ -104,7 +104,7 @@ static const struct woption long_options[] = {{L"background", required_argument, /// set_color builtin. maybe_t builtin_set_color(parser_t &parser, io_streams_t &streams, const wchar_t **argv) { // By the time this is called we should have initialized the curses subsystem. - assert(curses_initialized); + assert(CURSES_INITIALIZED); // Variables used for parsing the argument list. int argc = builtin_count_args(argv); diff --git a/src/env.cpp b/src/env.cpp index 9e868cb7f..fcd5c7d96 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -50,11 +50,6 @@ /// At init, we read all the environment variables from this array. extern char **environ; -bool curses_initialized = false; - -/// Does the terminal have the "eat_newline_glitch". -bool term_has_xn = false; - // static env_var_t env_var_t::new_ffi(EnvVar *ptr) { assert(ptr != nullptr && "env_var_t::new_ffi called with null pointer"); @@ -575,6 +570,7 @@ wcstring env_get_runtime_path() { static std::mutex s_setenv_lock{}; +extern "C" { void setenv_lock(const char *name, const char *value, int overwrite) { scoped_lock locker(s_setenv_lock); setenv(name, value, overwrite); @@ -584,6 +580,7 @@ void unsetenv_lock(const char *name) { scoped_lock locker(s_setenv_lock); unsetenv(name); } +} wcstring_list_ffi_t get_history_variable_text_ffi(const wcstring &fish_history_val) { wcstring_list_ffi_t out{}; diff --git a/src/env.h b/src/env.h index 8b0dc4410..c33a1a9c3 100644 --- a/src/env.h +++ b/src/env.h @@ -43,8 +43,14 @@ struct event_list_ffi_t { struct owning_null_terminated_array_t; -extern size_t read_byte_limit; -extern bool curses_initialized; +extern "C" { +extern bool CURSES_INITIALIZED; + +/// Does the terminal have the "eat_newline_glitch". +extern bool TERM_HAS_XN; + +extern size_t READ_BYTE_LIMIT; +} // Flags that may be passed as the 'mode' in env_stack_t::set() / environment_t::get(). enum : uint16_t { @@ -304,8 +310,6 @@ class env_stack_t final : public environment_t { bool get_use_posix_spawn(); -extern bool term_has_xn; // does the terminal have the "eat_newline_glitch" - /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); @@ -315,8 +319,10 @@ wcstring env_get_runtime_path(); /// A wrapper around setenv() and unsetenv() which use a lock. /// In general setenv() and getenv() are highly incompatible with threads. This makes it only /// slightly safer. +extern "C" { void setenv_lock(const char *name, const char *value, int overwrite); void unsetenv_lock(const char *name); +} /// Returns the originally inherited variables and their values. /// This is a simple key->value map and not e.g. cut into paths. diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index 58b3d3a23..bb7c7e51a 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -56,7 +56,6 @@ // Limit `read` to 100 MiB (bytes not wide chars) by default. This can be overridden by the // fish_read_limit variable. constexpr size_t DEFAULT_READ_BYTE_LIMIT = 100 * 1024 * 1024; -size_t read_byte_limit = DEFAULT_READ_BYTE_LIMIT; /// List of all locale environment variable names that might trigger (re)initializing the locale /// subsystem. These are only the variables we're possibly interested in. @@ -298,10 +297,10 @@ static void handle_read_limit_change(const environment_t &vars) { if (errno) { FLOGF(warning, "Ignoring fish_read_limit since it is not valid"); } else { - read_byte_limit = limit; + READ_BYTE_LIMIT = limit; } } else { - read_byte_limit = DEFAULT_READ_BYTE_LIMIT; + READ_BYTE_LIMIT = DEFAULT_READ_BYTE_LIMIT; } } @@ -617,12 +616,12 @@ static void init_curses(const environment_t &vars) { apply_term_hacks(vars); can_set_term_title = does_term_support_setting_title(vars); - term_has_xn = + TERM_HAS_XN = tigetflag(const_cast("xenl")) == 1; // does terminal have the eat_newline_glitch update_fish_color_support(vars); // Invalidate the cached escape sequences since they may no longer be valid. layout_cache_t::shared.clear(); - curses_initialized = true; + CURSES_INITIALIZED = true; } static constexpr const char *utf8_locales[] = { diff --git a/src/exec.cpp b/src/exec.cpp index 0b468972e..f69e2fb12 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1195,7 +1195,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, auto &ld = parser.libdata(); scoped_push is_subshell(&ld.is_subshell, true); - scoped_push read_limit(&ld.read_limit, is_subcmd ? read_byte_limit : 0); + scoped_push read_limit(&ld.read_limit, is_subcmd ? READ_BYTE_LIMIT : 0); auto prev_statuses = parser.get_last_statuses(); const cleanup_t put_back([&] { diff --git a/src/input.cpp b/src/input.cpp index 5af8202ba..7fc819920 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -854,7 +854,7 @@ std::shared_ptr input_mapping_set_t::all_mappings() { /// Create a list of terminfo mappings. static std::vector create_input_terminfo() { - assert(curses_initialized); + assert(CURSES_INITIALIZED); if (!cur_term) return {}; // setupterm() failed so we can't referency any key definitions #define TERMINFO_ADD(key) \ diff --git a/src/screen.cpp b/src/screen.cpp index 3b47df9ab..79c45fd29 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -1298,7 +1298,7 @@ void screen_t::reset_abandoning_line(int screen_width) { const_cast(exit_attribute_mode)))); // normal text ANSI escape sequence } - int newline_glitch_width = term_has_xn ? 0 : 1; + int newline_glitch_width = TERM_HAS_XN ? 0 : 1; abandon_line_string.append(screen_width - non_space_width - newline_glitch_width, L' '); } From c409b1a89cac3b8a9fca44e7d1183b722673c673 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 14:46:16 -0500 Subject: [PATCH 10/15] Port env_dispatch dependencies to rust Either add rust wrappers for C++ functions called via ffi or port some pure code from C++ to rust to provide support for the upcoming `env_dispatch` rewrite. --- fish-rust/src/lib.rs | 2 ++ fish-rust/src/output.rs | 19 +++++++++++++++++++ fish-rust/src/reader.rs | 15 +++++++++++++++ src/output.cpp | 7 ++++--- src/output.h | 6 ++++-- src/reader.cpp | 16 ++++++++++++++++ src/reader.h | 6 ++++++ src/screen.cpp | 4 ++++ src/screen.h | 2 ++ 9 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 fish-rust/src/output.rs create mode 100644 fish-rust/src/reader.rs diff --git a/fish-rust/src/lib.rs b/fish-rust/src/lib.rs index 74a68d00a..c98b0b4ee 100644 --- a/fish-rust/src/lib.rs +++ b/fish-rust/src/lib.rs @@ -44,12 +44,14 @@ mod locale; mod nix; mod null_terminated_array; mod operation_context; +mod output; mod parse_constants; mod parse_tree; mod parse_util; mod parser_keywords; mod path; mod re; +mod reader; mod redirection; mod signal; mod smoke; diff --git a/fish-rust/src/output.rs b/fish-rust/src/output.rs new file mode 100644 index 000000000..8f76c92f7 --- /dev/null +++ b/fish-rust/src/output.rs @@ -0,0 +1,19 @@ +use bitflags::bitflags; + +bitflags! { + pub struct ColorSupport: u8 { + const NONE = 0; + const TERM_256COLOR = 1<<0; + const TERM_24BIT = 1<<1; + } +} + +pub fn output_set_color_support(value: ColorSupport) { + extern "C" { + pub fn output_set_color_support(value: libc::c_int); + } + + unsafe { + output_set_color_support(value.bits() as i32); + } +} diff --git a/fish-rust/src/reader.rs b/fish-rust/src/reader.rs new file mode 100644 index 000000000..15ea8ac07 --- /dev/null +++ b/fish-rust/src/reader.rs @@ -0,0 +1,15 @@ +use crate::env::Environment; +use crate::wchar::L; + +#[repr(u8)] +pub enum CursorSelectionMode { + Exclusive = 0, + Inclusive = 1, +} + +pub fn check_autosuggestion_enabled(vars: &dyn Environment) -> bool { + vars.get(L!("fish_autosuggestion_enabled")) + .map(|v| v.as_string()) + .map(|v| v != L!("0")) + .unwrap_or(true) +} diff --git a/src/output.cpp b/src/output.cpp index da0ff8f99..bc494c2e6 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -41,9 +41,10 @@ static bool term_supports_color_natively(unsigned int c) { return static_cast(max_colors) >= c + 1; } -color_support_t output_get_color_support() { return color_support; } - -void output_set_color_support(color_support_t val) { color_support = val; } +extern "C" { + void output_set_color_support(color_support_t val) { color_support = val; } + color_support_t output_get_color_support() { return color_support; } +} unsigned char index_for_color(rgb_color_t c) { if (c.is_named() || !(output_get_color_support() & color_support_term256)) { diff --git a/src/output.h b/src/output.h index 2b786c31c..4aee22e6e 100644 --- a/src/output.h +++ b/src/output.h @@ -127,8 +127,10 @@ rgb_color_t parse_color(const env_var_t &var, bool is_background); /// Sets what colors are supported. enum { color_support_term256 = 1 << 0, color_support_term24bit = 1 << 1 }; using color_support_t = unsigned int; -color_support_t output_get_color_support(); -void output_set_color_support(color_support_t val); +extern "C" { + color_support_t output_get_color_support(); + void output_set_color_support(color_support_t val); +} rgb_color_t best_color(const std::vector &candidates, color_support_t support); diff --git a/src/reader.cpp b/src/reader.cpp index 0e25a45f6..436acff89 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -2922,6 +2922,10 @@ void reader_change_cursor_selection_mode(cursor_selection_mode_t selection_mode) } } +void reader_change_cursor_selection_mode(uint8_t selection_mode) { + reader_change_cursor_selection_mode((cursor_selection_mode_t) selection_mode); +} + static bool check_autosuggestion_enabled(const env_stack_t &vars) { if (auto val = vars.get(L"fish_autosuggestion_enabled")) { return val->as_string() != L"0"; @@ -2942,6 +2946,18 @@ void reader_set_autosuggestion_enabled(const env_stack_t &vars) { } } +void reader_set_autosuggestion_enabled_ffi(bool enable) { + // We don't need to _change_ if we're not initialized yet. + reader_data_t *data = current_data_or_null(); + if (data) { + if (data->conf.autosuggest_ok != enable) { + data->conf.autosuggest_ok = enable; + data->force_exec_prompt_and_repaint = true; + data->inputter.queue_char(readline_cmd_t::repaint); + } + } +} + /// Add a new reader to the reader stack. /// \return a shared pointer to it. static std::shared_ptr reader_push_ret(parser_t &parser, diff --git a/src/reader.h b/src/reader.h index 4d282be28..9adc8467d 100644 --- a/src/reader.h +++ b/src/reader.h @@ -168,11 +168,17 @@ enum class cursor_selection_mode_t : uint8_t { inclusive, }; +#if INCLUDE_RUST_HEADERS void reader_change_cursor_selection_mode(cursor_selection_mode_t selection_mode); +#else +void reader_change_cursor_selection_mode(uint8_t selection_mode); +#endif /// Enable or disable autosuggestions based on the associated variable. void reader_set_autosuggestion_enabled(const env_stack_t &vars); +void reader_set_autosuggestion_enabled_ffi(bool); + /// Write the title to the titlebar. This function is called just before a new application starts /// executing and just after it finishes. /// diff --git a/src/screen.cpp b/src/screen.cpp index 79c45fd29..afb5eb08f 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -63,6 +63,10 @@ class scoped_buffer_t : noncopyable_t, nonmovable_t { // Note this is deliberately exported so that init_curses can clear it. layout_cache_t layout_cache_t::shared; +void screen_clear_layout_cache_ffi() { + layout_cache_t::shared.clear(); +} + /// Tests if the specified narrow character sequence is present at the specified position of the /// specified wide character string. All of \c seq must match, but str may be longer than seq. static size_t try_sequence(const char *seq, const wchar_t *str) { diff --git a/src/screen.h b/src/screen.h index 4c667baa6..d215fa4da 100644 --- a/src/screen.h +++ b/src/screen.h @@ -244,6 +244,8 @@ class screen_t { /// Issues an immediate clr_eos. void screen_force_clear_to_end(); +void screen_clear_layout_cache_ffi(); + // Information about the layout of a prompt. struct prompt_layout_t { std::vector line_breaks; // line breaks when rendering the prompt From c71342b933ddd11ee89917016fb809f03b565e03 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 15:12:22 -0500 Subject: [PATCH 11/15] Add safe Rust wrapper around system curses library This is not yet used but will take eventually take the place of all (n)curses access. The curses C library does a lot of header file magic with macro voodoo to make it easier to perform certain tasks (such as access or override string capabilities) but this functionality isn't actually directly exposed by the library's ABI. The rust wrapper eschews all of that for a more straight-forward implementation, directly wrapping only the basic curses library calls that are required to perform the tasks we care about. This should let us avoid the subtle cross-platform differences between the various curses implementations that plagued the previous C++ implementation. All functionality in this module that requires an initialized curses TERMINAL pointer (`cur_term`, traditionally) has been subsumed by the `Term` instance, which once initialized with `curses::setup()` can be obtained at any time with `curses::Term()` (which returns an Option that evaluates to `None` if `cur_term` hasn't yet been initialized). --- fish-rust/src/curses.rs | 273 ++++++++++++++++++++++++++++++++++++++++ fish-rust/src/lib.rs | 1 + 2 files changed, 274 insertions(+) create mode 100644 fish-rust/src/curses.rs diff --git a/fish-rust/src/curses.rs b/fish-rust/src/curses.rs new file mode 100644 index 000000000..37de139bb --- /dev/null +++ b/fish-rust/src/curses.rs @@ -0,0 +1,273 @@ +//! A wrapper around the system's curses/ncurses library, exposing some lower-level functionality +//! that's not directly exposed in any of the popular ncurses crates. +//! +//! In addition to exposing the C library ffi calls, we also shim around some functionality that's +//! only made available via the the ncurses headers to C code via macro magic, such as polyfilling +//! missing capability strings to shoe-in missing support for certain terminal sequences. +//! +//! This is intentionally very bare bones and only implements the subset of curses functionality +//! used by fish + +use self::sys::*; +use std::ffi::{CStr, CString}; +use std::sync::Arc; +use std::sync::Mutex; + +/// The [`Term`] singleton, providing a façade around the system curses library. Initialized via a +/// successful call to [`setup()`] and surfaced to the outside world via [`term()`]. +/// +/// It isn't guaranteed that fish will ever be able to successfully call `setup()`, so this must +/// remain an `Option` instead of returning `Term` by default and just panicking if [`term()`] was +/// called before `setup()`. +/// +/// We can't just use an AtomicPtr> here because there's a race condition when the old Arc +/// gets dropped - we would obtain the current (non-null) value of `TERM` in [`term()`] but there's +/// no guarantee that a simultaneous call to [`setup()`] won't result in this refcount being +/// decremented to zero and the memory being reclaimed before we can clone it, since we can only +/// atomically *read* the value of the pointer, not clone the `Arc` it points to. +pub static TERM: Mutex>> = Mutex::new(None); + +/// Returns a reference to the global [`Term`] singleton or `None` if not preceded by a successful +/// call to [`curses::setup()`]. +pub fn term() -> Option> { + TERM.lock() + .expect("Mutex poisoned!") + .as_ref() + .map(Arc::clone) +} + +/// Private module exposing system curses ffi. +mod sys { + pub const OK: i32 = 0; + pub const ERR: i32 = -1; + + extern "C" { + /// The ncurses `cur_term` TERMINAL pointer. + pub static mut cur_term: *const core::ffi::c_void; + + /// setupterm(3) is a low-level call to begin doing any sort of `term.h`/`curses.h` work. + /// It's called internally by ncurses's `initscr()` and `newterm()`, but the C++ code called + /// it directly from [`initialize_curses_using_fallbacks()`]. + pub fn setupterm( + term: *const libc::c_char, + filedes: libc::c_int, + errret: *mut libc::c_int, + ) -> libc::c_int; + + /// Frees the `cur_term` TERMINAL pointer. + pub fn del_curterm(term: *const core::ffi::c_void) -> libc::c_int; + + /// Checks for the presence of a termcap flag identified by the first two characters of + /// `id`. + pub fn tgetflag(id: *const libc::c_char) -> libc::c_int; + + /// Checks for the presence and value of a number capability in the termcap/termconf + /// database. A return value of `-1` indicates not found. + pub fn tgetnum(id: *const libc::c_char) -> libc::c_int; + + pub fn tgetstr( + id: *const libc::c_char, + area: *mut *mut libc::c_char, + ) -> *const libc::c_char; + } +} + +/// The safe wrapper around curses functionality, initialized by a successful call to [`setup()`] +/// and obtained thereafter by calls to [`term()`]. +/// +/// An extant `Term` instance means the curses `TERMINAL *cur_term` pointer is non-null. Any +/// functionality that is normally performed using `cur_term` should be done via `Term` instead. +pub struct Term { + // String capabilities + pub enter_italics_mode: Option, + pub exit_italics_mode: Option, + pub enter_dim_mode: Option, + + // Number capabilities + pub max_colors: Option, + + // Flag/boolean capabilities + pub eat_newline_glitch: bool, +} + +impl Term { + /// Initialize a new `Term` instance, prepopulating the values of all the curses string + /// capabilities we care about in the process. + fn new() -> Self { + Term { + // String capabilities + enter_italics_mode: StringCap::new("ZH").lookup(), + exit_italics_mode: StringCap::new("ZR").lookup(), + enter_dim_mode: StringCap::new("mh").lookup(), + + // Number capabilities + max_colors: NumberCap::new("Co").lookup(), + + // Flag/boolean capabilities + eat_newline_glitch: FlagCap::new("xn").lookup(), + } + } +} + +trait Capability { + type Result: Sized; + fn lookup(&self) -> Self::Result; +} + +impl Capability for StringCap { + type Result = Option; + + fn lookup(&self) -> Self::Result { + unsafe { + const NULL: *const i8 = core::ptr::null(); + match sys::tgetstr(self.code.as_ptr(), core::ptr::null_mut()) { + NULL => None, + // termcap spec says nul is not allowed in terminal sequences and must be encoded; + // so the terminating NUL is the end of the string. + result => Some(CStr::from_ptr(result).to_owned()), + } + } + } +} + +impl Capability for NumberCap { + type Result = Option; + + fn lookup(&self) -> Self::Result { + unsafe { + match tgetnum(self.0.as_ptr()) { + -1 => None, + n => Some(n), + } + } + } +} + +impl Capability for FlagCap { + type Result = bool; + + fn lookup(&self) -> Self::Result { + unsafe { tgetflag(self.0.as_ptr()) != 0 } + } +} + +/// Calls the curses `setupterm()` function with the provided `$TERM` value `term` (or a null +/// pointer in case `term` is null) for the file descriptor `fd`. Returns a reference to the newly +/// initialized [`Term`] singleton on success or `None` if this failed. +/// +/// The `configure` parameter may be set to a callback that takes an `&mut Term` reference to +/// override any capabilities before the `Term` is permanently made immutable. +/// +/// Note that the `errret` parameter is provided to the function, meaning curses will not write +/// error output to stderr in case of failure. +/// +/// Any existing references from `curses::term()` will be invalidated by this call! +pub fn setup(term: Option<&CStr>, fd: i32, configure: F) -> Option> +where + F: Fn(&mut Term), +{ + // For now, use the same TERM lock when using `cur_term` to prevent any race conditions in + // curses itself. We might split this to another lock in the future. + let mut global_term = TERM.lock().expect("Mutex poisoned!"); + + let result = unsafe { + // If cur_term is already initialized for a different $TERM value, calling setupterm() again + // will leak memory. Call del_curterm() first to free previously allocated resources. + let _ = sys::del_curterm(cur_term); + + let mut err = 0; + if let Some(term) = term { + sys::setupterm(term.as_ptr(), fd, &mut err) + } else { + sys::setupterm(core::ptr::null(), fd, &mut err) + } + }; + + // Safely store the new Term instance or replace the old one. We have the lock so it's safe to + // drop the old TERM value and have its refcount decremented - no one will be cloning it. + if result == sys::OK { + // Create a new `Term` instance, prepopulate the capabilities we care about, and allow the + // caller to override any as needed. + let mut term = Term::new(); + (configure)(&mut term); + + let term = Arc::new(term); + *global_term = Some(term.clone()); + Some(term) + } else { + *global_term = None; + None + } +} + +/// Resets the curses `cur_term` TERMINAL pointer. Subsequent calls to [`curses::term()`](term()) +/// will return `None`. +pub fn reset() { + let mut term = TERM.lock().expect("Mutex poisoned!"); + if term.is_some() { + unsafe { + // Ignore the result of del_curterm() as the only documented error is that + // `cur_term` was already null. + let _ = sys::del_curterm(cur_term); + sys::cur_term = core::ptr::null(); + } + *term = None; + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +struct Code { + /// The two-char termcap code for the capability, followed by a nul. + code: [u8; 3], +} + +impl Code { + /// `code` is the two-digit termcap code. See termcap(5) for a reference. + /// + /// Panics if anything other than a two-ascii-character `code` is passed into the function. It + /// would take a hard-coded `[u8; 2]` parameter but that is less ergonomic. Since all our + /// termcap `Code`s are compile-time constants, the panic is a compile-time error, meaning + /// there's no harm to going this more ergonomic route. + const fn new(code: &str) -> Code { + let code = code.as_bytes(); + if code.len() != 2 { + panic!("Invalid termcap code provided!"); + } + Code { + code: [code[0], code[1], b'\0'], + } + } + + /// The nul-terminated termcap id of the capability. + pub const fn as_ptr(&self) -> *const libc::c_char { + self.code.as_ptr().cast() + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +struct StringCap { + code: Code, +} +impl StringCap { + const fn new(code: &str) -> Self { + StringCap { + code: Code::new(code), + } + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +struct NumberCap(Code); +impl NumberCap { + const fn new(code: &str) -> Self { + NumberCap(Code::new(code)) + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +struct FlagCap(Code); +impl FlagCap { + const fn new(code: &str) -> Self { + FlagCap(Code::new(code)) + } +} diff --git a/fish-rust/src/lib.rs b/fish-rust/src/lib.rs index c98b0b4ee..612c113f9 100644 --- a/fish-rust/src/lib.rs +++ b/fish-rust/src/lib.rs @@ -18,6 +18,7 @@ mod ast; mod builtins; mod color; mod compat; +mod curses; mod env; mod event; mod expand; From 6bb2725f67c114273c3f9b5f24376145f825c96e Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 17:22:16 -0500 Subject: [PATCH 12/15] Make sure rust's fish_setlocale() inits global C++ variables We can't just call the Rust version of `fish_setlocale()` without also either calling the C++ version of `fish_setlocale()` or removing all `src/complete.cpp` variables that are initialized and aliasing them to their new rust counterparts. Since we're not interested in keeping the C++ code around, just call the C++ version of the function via ffi until we don't have *any* C++ code referencing `src/common.h` at all. Note that *not* doing this and then calling the rust version of `fish_setlocale()` instead of the C++ version will cause errant behavior and random segfaults as the C++ code will try to read and use uninitialized values (including uninitialized pointers) that have only had their rust counterparts init. --- fish-rust/src/common.rs | 11 ++++++++++- src/common.cpp | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/fish-rust/src/common.rs b/fish-rust/src/common.rs index 51e9dac8e..59d7f6599 100644 --- a/fish-rust/src/common.rs +++ b/fish-rust/src/common.rs @@ -1345,7 +1345,7 @@ pub type FilenameRef = Rc; /// This function should be called after calling `setlocale()` to perform fish specific locale /// initialization. #[widestrs] -fn fish_setlocale() { +pub fn fish_setlocale() { // Use various Unicode symbols if they can be encoded using the current locale, else a simple // ASCII char alternative. All of the can_be_encoded() invocations should return the same // true/false value since the code points are in the BMP but we're going to be paranoid. This @@ -1395,6 +1395,15 @@ fn fish_setlocale() { ); } PROFILING_ACTIVE.store(true); + + // Until no C++ code uses the variables init in the C++ version of fish_setlocale(), we need to + // also call that one or otherwise we'll segfault trying to read those uninit values. + extern "C" { + fn fish_setlocale_ffi(); + } + unsafe { + fish_setlocale_ffi(); + } } /// Test if the character can be encoded using the current locale. diff --git a/src/common.cpp b/src/common.cpp index ffc0b2e23..53e8684c2 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1572,3 +1572,12 @@ bool is_console_session() { }(); return console_session; } + +/// Expose the C++ version of fish_setlocale as fish_setlocale_ffi so the variables we initialize +/// can be init even if the rust version of the function is called instead. This is easier than +/// declaring all those variables as extern, which I'll do in a separate PR. +extern "C" { + void fish_setlocale_ffi() { + fish_setlocale(); + } +} From 32912b6525ba74f941f7121f4578550c8205c1a3 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 18:02:18 -0500 Subject: [PATCH 13/15] Expose `env_dyn_t` in `env.h` So that we may use it from files other than `src/env.cpp` to accept a `&dyn Environment` out of rust. --- src/env.cpp | 29 ++++++++++------------------- src/env.h | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index fcd5c7d96..d60e6c928 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -450,27 +450,18 @@ std::shared_ptr env_stack_t::export_arr() { rust::Box::from_raw(ptr)); } -/// Wrapper around a EnvDyn. -class env_dyn_t final : public environment_t { - public: - env_dyn_t(rust::Box impl) : impl_(std::move(impl)) {} - - maybe_t get(const wcstring &key, env_mode_flags_t mode) const { - if (auto *ptr = impl_->getf(key, mode)) { - return env_var_t::new_ffi(ptr); - } - return none(); +maybe_t env_dyn_t::get(const wcstring &key, env_mode_flags_t mode) const { + if (auto *ptr = impl_->getf(key, mode)) { + return env_var_t::new_ffi(ptr); } + return none(); +} - std::vector get_names(env_mode_flags_t flags) const { - wcstring_list_ffi_t names; - impl_->get_names(flags, names); - return std::move(names.vals); - } - - private: - rust::Box impl_; -}; +std::vector env_dyn_t::get_names(env_mode_flags_t flags) const { + wcstring_list_ffi_t names; + impl_->get_names(flags, names); + return std::move(names.vals); +} std::shared_ptr env_stack_t::snapshot() const { auto res = std::make_shared(impl_->snapshot()); diff --git a/src/env.h b/src/env.h index c33a1a9c3..5cdda13d8 100644 --- a/src/env.h +++ b/src/env.h @@ -313,6 +313,21 @@ bool get_use_posix_spawn(); /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); +#if INCLUDE_RUST_HEADERS +struct EnvDyn; +/// Wrapper around rust's `&dyn Environment` deriving from `environment_t`. +class env_dyn_t final : public environment_t { + public: + env_dyn_t(rust::Box impl) : impl_(std::move(impl)) {} + maybe_t get(const wcstring &key, env_mode_flags_t mode) const; + + std::vector get_names(env_mode_flags_t flags) const; + + private: + rust::Box impl_; +}; +#endif + /// Gets a path appropriate for runtime storage wcstring env_get_runtime_path(); From cce78eeb4339bcba704609e0ac8364d546d9c028 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 20:11:05 -0500 Subject: [PATCH 14/15] Update env_var_to_ffi() to take an Option It wasn't possible to handle cases where vars.get() returned `None` then forward that to C++, but now we can. --- fish-rust/src/env/environment.rs | 8 ++++++-- fish-rust/src/flog.rs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/fish-rust/src/env/environment.rs b/fish-rust/src/env/environment.rs index 0a0b30697..b92512984 100644 --- a/fish-rust/src/env/environment.rs +++ b/fish-rust/src/env/environment.rs @@ -34,8 +34,12 @@ lazy_static! { static UVARS_LOCALLY_MODIFIED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); /// Convert an EnvVar to an FFI env_var_t. -fn env_var_to_ffi(var: EnvVar) -> cxx::UniquePtr { - ffi::env_var_t::new_ffi(Box::into_raw(Box::from(var)).cast()).within_unique_ptr() +pub fn env_var_to_ffi(var: Option) -> cxx::UniquePtr { + if let Some(var) = var { + ffi::env_var_t::new_ffi(Box::into_raw(Box::from(var)).cast()).within_unique_ptr() + } else { + cxx::UniquePtr::null() + } } /// An environment is read-only access to variable values. diff --git a/fish-rust/src/flog.rs b/fish-rust/src/flog.rs index 9add0b6cd..56232c786 100644 --- a/fish-rust/src/flog.rs +++ b/fish-rust/src/flog.rs @@ -150,7 +150,7 @@ pub trait FloggableDisplay { impl FloggableDisplay for T { fn to_flog_str(&self) -> String { - format!("{}", self) + self.to_string() } } From 6638c78b30310c9c0b235e989827d9639a7897c4 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 20:51:34 -0500 Subject: [PATCH 15/15] Port env_dispatch to Rust and integrate with C++ code --- CMakeLists.txt | 2 +- fish-rust/build.rs | 9 +- fish-rust/src/env/environment.rs | 17 +- fish-rust/src/env_dispatch.rs | 820 +++++++++++++++++++++++++++++++ fish-rust/src/ffi.rs | 19 +- fish-rust/src/lib.rs | 1 + fish-rust/src/termsize.rs | 48 +- fish-rust/src/threads.rs | 17 +- src/env.cpp | 4 +- src/env.h | 5 - src/env_dispatch.h | 22 - src/exec.cpp | 3 +- src/history.cpp | 11 +- src/history.h | 6 + src/input_common.cpp | 17 + src/input_common.h | 1 + src/reader.cpp | 1 + src/reader.h | 4 +- 18 files changed, 938 insertions(+), 69 deletions(-) create mode 100644 fish-rust/src/env_dispatch.rs delete mode 100644 src/env_dispatch.h diff --git a/CMakeLists.txt b/CMakeLists.txt index ea6b3d106..81de7c106 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,7 @@ set(FISH_BUILTIN_SRCS # List of other sources. set(FISH_SRCS src/ast.cpp src/autoload.cpp src/color.cpp src/common.cpp src/complete.cpp - src/env.cpp src/env_dispatch.cpp src/env_universal_common.cpp src/event.cpp + src/env.cpp src/env_universal_common.cpp src/event.cpp src/exec.cpp src/expand.cpp src/fallback.cpp src/fish_indent_common.cpp src/fish_version.cpp src/flog.cpp src/function.cpp src/highlight.cpp src/history.cpp src/history_file.cpp src/input.cpp src/input_common.cpp diff --git a/fish-rust/build.rs b/fish-rust/build.rs index cc661871a..5bc37f7ec 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -40,9 +40,11 @@ fn main() { let source_files = vec![ "src/abbrs.rs", "src/ast.rs", - "src/env/env_ffi.rs", - "src/event.rs", + "src/builtins/shared.rs", "src/common.rs", + "src/env/env_ffi.rs", + "src/env_dispatch.rs", + "src/event.rs", "src/fd_monitor.rs", "src/fd_readable_set.rs", "src/fds.rs", @@ -60,14 +62,13 @@ fn main() { "src/signal.rs", "src/smoke.rs", "src/termsize.rs", + "src/threads.rs", "src/timer.rs", "src/tokenizer.rs", "src/topic_monitor.rs", - "src/threads.rs", "src/trace.rs", "src/util.rs", "src/wait_handle.rs", - "src/builtins/shared.rs", ]; cxx_build::bridges(&source_files) .flag_if_supported("-std=c++11") diff --git a/fish-rust/src/env/environment.rs b/fish-rust/src/env/environment.rs index b92512984..e904b46a7 100644 --- a/fish-rust/src/env/environment.rs +++ b/fish-rust/src/env/environment.rs @@ -5,6 +5,7 @@ use super::environment_impl::{ use crate::abbrs::{abbrs_get_set, Abbreviation, Position}; use crate::common::{unescape_string, UnescapeStringStyle}; use crate::env::{EnvMode, EnvStackSetResult, EnvVar, Statuses}; +use crate::env_dispatch::env_dispatch_var_change; use crate::event::Event; use crate::ffi::{self, env_universal_t, universal_notifier_t}; use crate::flog::FLOG; @@ -12,7 +13,7 @@ use crate::global_safety::RelaxedAtomicBool; use crate::null_terminated_array::OwningNullTerminatedArray; use crate::path::path_make_canonical; use crate::wchar::{wstr, WExt, WString, L}; -use crate::wchar_ffi::{AsWstr, WCharFromFFI, WCharToFFI}; +use crate::wchar_ffi::{AsWstr, WCharFromFFI}; use crate::wcstringutil::join_strings; use crate::wutil::{wgetcwd, wgettext}; @@ -184,7 +185,7 @@ impl EnvStack { // If we modified the global state, or we are principal, then dispatch changes. // Important to not hold the lock here. if ret.global_modified || self.is_principal() { - ffi::env_dispatch_var_change_ffi(&key.to_ffi() /* , self */); + env_dispatch_var_change(key, self); } } // Mark if we modified a uvar. @@ -232,7 +233,7 @@ impl EnvStack { if ret.status == EnvStackSetResult::ENV_OK { if ret.global_modified || self.is_principal() { // Important to not hold the lock here. - ffi::env_dispatch_var_change_ffi(&key.to_ffi() /*, self */); + env_dispatch_var_change(key, self); } } if ret.uvar_modified { @@ -259,7 +260,7 @@ impl EnvStack { // TODO: we would like to coalesce locale / curses changes, so that we only re-initialize // once. for key in popped { - ffi::env_dispatch_var_change_ffi(&key.to_ffi() /*, self */); + env_dispatch_var_change(&key, self); } } } @@ -302,7 +303,7 @@ impl EnvStack { #[allow(unreachable_code)] for idx in 0..sync_res.count() { let name = sync_res.get_key(idx).from_ffi(); - ffi::env_dispatch_var_change_ffi(&name.to_ffi() /* , self */); + env_dispatch_var_change(&name, self); let evt = if sync_res.get_is_erase(idx) { Event::variable_erase(name) } else { @@ -375,17 +376,17 @@ pub fn env_init(do_uvars: bool) { if !do_uvars { UVAR_SCOPE_IS_GLOBAL.store(true); } else { - // let vars = EnvStack::principal(); - // Set up universal variables using the default path. let callbacks = uvars() .as_mut() .unwrap() .initialize_ffi() .within_unique_ptr(); + let vars = EnvStack::principal(); let callbacks = callbacks.as_ref().unwrap(); for idx in 0..callbacks.count() { - ffi::env_dispatch_var_change_ffi(callbacks.get_key(idx) /* , vars */); + let name = callbacks.get_key(idx).from_ffi(); + env_dispatch_var_change(&name, vars); } // Do not import variables that have the same name and value as diff --git a/fish-rust/src/env_dispatch.rs b/fish-rust/src/env_dispatch.rs new file mode 100644 index 000000000..4e40fb3e9 --- /dev/null +++ b/fish-rust/src/env_dispatch.rs @@ -0,0 +1,820 @@ +use crate::common::ToCString; +use crate::curses::{self, Term}; +use crate::env::{setenv_lock, unsetenv_lock, EnvMode, EnvStack, Environment}; +use crate::env::{CURSES_INITIALIZED, READ_BYTE_LIMIT, TERM_HAS_XN}; +use crate::ffi::is_interactive_session; +use crate::flog::FLOGF; +use crate::output::ColorSupport; +use crate::wchar::L; +use crate::wchar::{wstr, WString}; +use crate::wchar_ext::WExt; +use crate::wutil::fish_wcstoi; +use crate::wutil::wgettext; +use std::collections::HashMap; +use std::ffi::{CStr, CString}; +use std::sync::atomic::{AtomicBool, Ordering}; + +#[cxx::bridge] +mod env_dispatch_ffi { + extern "Rust" { + fn env_dispatch_init_ffi(); + fn term_supports_setting_title() -> bool; + fn use_posix_spawn() -> bool; + } +} + +/// List of all locale environment variable names that might trigger (re)initializing of the locale +/// subsystem. These are only the variables we're possibly interested in. +#[rustfmt::skip] +const LOCALE_VARIABLES: [&wstr; 10] = [ + L!("LANG"), L!("LANGUAGE"), L!("LC_ALL"), + L!("LC_COLLATE"), L!("LC_CTYPE"), L!("LC_MESSAGES"), + L!("LC_NUMERIC"), L!("LC_TIME"), L!("LOCPATH"), + L!("fish_allow_singlebyte_locale"), +]; + +#[rustfmt::skip] +const CURSES_VARIABLES: [&wstr; 3] = [ + L!("TERM"), L!("TERMINFO"), L!("TERMINFO_DIRS") +]; + +/// Whether to use `posix_spawn()` when possible. +static USE_POSIX_SPAWN: AtomicBool = AtomicBool::new(false); + +/// Whether we think we can set the terminal title or not. +static CAN_SET_TERM_TITLE: AtomicBool = AtomicBool::new(false); + +/// The variable dispatch table. This is set at startup and cannot be modified after. +static VAR_DISPATCH_TABLE: once_cell::sync::Lazy = + once_cell::sync::Lazy::new(|| { + let mut table = VarDispatchTable::default(); + + for name in LOCALE_VARIABLES { + table.add_anon(name, handle_locale_change); + } + + for name in CURSES_VARIABLES { + table.add_anon(name, handle_curses_change); + } + + table.add(L!("TZ"), handle_tz_change); + table.add_anon(L!("fish_term256"), handle_fish_term_change); + table.add_anon(L!("fish_term24bit"), handle_fish_term_change); + table.add_anon(L!("fish_escape_delay_ms"), update_wait_on_escape_ms); + table.add_anon(L!("fish_emoji_width"), guess_emoji_width); + table.add_anon(L!("fish_ambiguous_width"), handle_change_ambiguous_width); + table.add_anon(L!("LINES"), handle_term_size_change); + table.add_anon(L!("COLUMNS"), handle_term_size_change); + table.add_anon(L!("fish_complete_path"), handle_complete_path_change); + table.add_anon(L!("fish_function_path"), handle_function_path_change); + table.add_anon(L!("fish_read_limit"), handle_read_limit_change); + table.add_anon(L!("fish_history"), handle_fish_history_change); + table.add_anon( + L!("fish_autosuggestion_enabled"), + handle_autosuggestion_change, + ); + table.add_anon( + L!("fish_use_posix_spawn"), + handle_fish_use_posix_spawn_change, + ); + table.add_anon(L!("fish_trace"), handle_fish_trace); + table.add_anon( + L!("fish_cursor_selection_mode"), + handle_fish_cursor_selection_mode_change, + ); + + table + }); + +type NamedEnvCallback = fn(name: &wstr, env: &EnvStack); +type AnonEnvCallback = fn(env: &EnvStack); + +#[derive(Default)] +struct VarDispatchTable { + named_table: HashMap<&'static wstr, NamedEnvCallback>, + anon_table: HashMap<&'static wstr, AnonEnvCallback>, +} + +// TODO: Delete this after input_common is ported (and pass the input_function function directly). +fn update_wait_on_escape_ms(vars: &EnvStack) { + let fish_escape_delay_ms = vars.get_unless_empty(L!("fish_escape_delay_ms")); + let var = crate::env::environment::env_var_to_ffi(fish_escape_delay_ms); + crate::ffi::update_wait_on_escape_ms_ffi(var); +} + +impl VarDispatchTable { + fn observes_var(&self, name: &wstr) -> bool { + self.named_table.contains_key(name) || self.anon_table.contains_key(name) + } + + /// Add a callback for the variable `name`. We must not already be observing this variable. + pub fn add(&mut self, name: &'static wstr, callback: NamedEnvCallback) { + let prev = self.named_table.insert(name, callback); + assert!( + prev.is_none() && !self.anon_table.contains_key(name), + "Already observing {}", + name + ); + } + + /// Add an callback for the variable `name`. We must not already be observing this variable. + pub fn add_anon(&mut self, name: &'static wstr, callback: AnonEnvCallback) { + let prev = self.anon_table.insert(name, callback); + assert!( + prev.is_none() && !self.named_table.contains_key(name), + "Already observing {}", + name + ); + } + + pub fn dispatch(&self, key: &wstr, vars: &EnvStack) { + if let Some(named) = self.named_table.get(key) { + (named)(key, vars); + } + if let Some(anon) = self.anon_table.get(key) { + (anon)(vars); + } + } +} + +fn handle_timezone(var_name: &wstr, vars: &EnvStack) { + let var = vars.get_unless_empty(var_name).map(|v| v.as_string()); + FLOGF!( + env_dispatch, + "handle_timezone() current timezone var:", + var_name, + "=>", + var.as_ref() + .map(|v| v.as_utfstr()) + .unwrap_or(L!("MISSING/EMPTY")), + ); + if let Some(value) = var { + setenv_lock(var_name, &value, true); + } else { + unsetenv_lock(var_name); + } + + extern "C" { + fn tzset(); + } + + unsafe { + tzset(); + } +} + +/// Update the value of [`FISH_EMOJI_WIDTH`]. +fn guess_emoji_width(vars: &EnvStack) { + use crate::fallback::FISH_EMOJI_WIDTH; + + if let Some(width_str) = vars.get(L!("fish_emoji_width")) { + // The only valid values are 1 or 2; we default to 1 if it was an invalid int. + let new_width = fish_wcstoi(&width_str.as_string()).unwrap_or(1).clamp(1, 2); + FISH_EMOJI_WIDTH.store(new_width, Ordering::Relaxed); + FLOGF!( + term_support, + "Overriding default fish_emoji_width w/", + new_width + ); + return; + } + + let term = vars + .get(L!("TERM_PROGRAM")) + .map(|v| v.as_string()) + .unwrap_or_else(WString::new); + // The format and contents of $TERM_PROGRAM_VERSION depend on $TERM_PROGRAM. Under + // Apple_Terminal, this is an integral value in the hundreds corresponding to the + // CFBundleVersion of Terminal.app; under iTerm, this is the version number which can contain + // multiple periods (e.g 3.4.19). Currently we only care about Apple_Terminal but the C++ code + // used wcstod() to parse at least the major.minor value of cases like the latter. + // + // TODO: Move this inside the Apple_Terminal branch and use i32::FromStr (i.e. str::parse()) + // instead. + let version = vars + .get(L!("TERM_PROGRAM_VERSION")) + .map(|v| v.as_string()) + .and_then(|v| { + let mut consumed = 0; + crate::wutil::wcstod::wcstod(&v, '.', &mut consumed).ok() + }) + .unwrap_or(0.0); + + if term == "Apple_Terminal" && version as i32 >= 400 { + // Apple Terminal on High Sierra + FISH_EMOJI_WIDTH.store(2, Ordering::Relaxed); + FLOGF!(term_support, "default emoji width: 2 for", term); + } else if term == "iTerm.app" { + // iTerm2 now defaults to Unicode 9 sizes for anything after macOS 10.12 + FISH_EMOJI_WIDTH.store(2, Ordering::Relaxed); + FLOGF!(term_support, "default emoji width 2 for iTerm2"); + } else { + // Default to whatever the system's wcwidth gives for U+1F603, but only if it's at least + // 1 and at most 2. + let width = crate::fallback::wcwidth('😃').clamp(1, 2); + FISH_EMOJI_WIDTH.store(width, Ordering::Relaxed); + FLOGF!(term_support, "default emoji width:", width); + } +} + +/// React to modifying the given variable. +pub fn env_dispatch_var_change(key: &wstr, vars: &EnvStack) { + use once_cell::sync::Lazy; + + // We want to ignore variable changes until the dispatch table is explicitly initialized. + if let Some(dispatch_table) = Lazy::get(&VAR_DISPATCH_TABLE) { + dispatch_table.dispatch(key, vars); + } +} + +fn handle_fish_term_change(vars: &EnvStack) { + update_fish_color_support(vars); + crate::ffi::reader_schedule_prompt_repaint(); +} + +fn handle_change_ambiguous_width(vars: &EnvStack) { + let new_width = vars + .get(L!("fish_ambiguous_width")) + .map(|v| v.as_string()) + // We use the default value of 1 if it was an invalid int. + .and_then(|fish_ambiguous_width| fish_wcstoi(&fish_ambiguous_width).ok()) + .unwrap_or(1) + // Clamp in case of negative values. + .max(0); + crate::fallback::FISH_AMBIGUOUS_WIDTH.store(new_width, Ordering::Relaxed); +} + +fn handle_term_size_change(vars: &EnvStack) { + crate::termsize::handle_columns_lines_var_change(vars); +} + +fn handle_fish_history_change(vars: &EnvStack) { + let fish_history = vars.get(L!("fish_history")); + let var = crate::env::env_var_to_ffi(fish_history); + crate::ffi::reader_change_history(&crate::ffi::history_session_id(var)); +} + +fn handle_fish_cursor_selection_mode_change(vars: &EnvStack) { + use crate::reader::CursorSelectionMode; + + let inclusive = vars + .get(L!("fish_cursor_selection_mode")) + .as_ref() + .map(|v| v.as_string()) + .map(|v| v == "inclusive") + .unwrap_or(false); + let mode = if inclusive { + CursorSelectionMode::Inclusive + } else { + CursorSelectionMode::Exclusive + }; + + let mode = mode as u8; + crate::ffi::reader_change_cursor_selection_mode(mode); +} + +fn handle_autosuggestion_change(vars: &EnvStack) { + // TODO: This was a call to reader_set_autosuggestion_enabled(vars) and + // reader::check_autosuggestion_enabled() should be private to the `reader` module. + crate::ffi::reader_set_autosuggestion_enabled_ffi(crate::reader::check_autosuggestion_enabled( + vars, + )); +} + +fn handle_function_path_change(_: &EnvStack) { + crate::ffi::function_invalidate_path(); +} + +fn handle_complete_path_change(_: &EnvStack) { + crate::ffi::complete_invalidate_path(); +} + +fn handle_tz_change(var_name: &wstr, vars: &EnvStack) { + handle_timezone(var_name, vars); +} + +fn handle_locale_change(vars: &EnvStack) { + init_locale(vars); + // We need to re-guess emoji width because the locale might have changed to a multibyte one. + guess_emoji_width(vars); +} + +fn handle_curses_change(vars: &EnvStack) { + guess_emoji_width(vars); + init_curses(vars); +} + +fn handle_fish_use_posix_spawn_change(vars: &EnvStack) { + // Note that if the variable is missing or empty we default to true (if allowed). + if !allow_use_posix_spawn() { + USE_POSIX_SPAWN.store(false, Ordering::Relaxed); + } else if let Some(var) = vars.get(L!("fish_use_posix_spawn")) { + let use_posix_spawn = + var.is_empty() || crate::wcstringutil::bool_from_string(&var.as_string()); + USE_POSIX_SPAWN.store(use_posix_spawn, Ordering::Relaxed); + } else { + USE_POSIX_SPAWN.store(true, Ordering::Relaxed); + } +} + +/// Allow the user to override the limits on how much data the `read` command will process. This is +/// primarily intended for testing, but could also be used directly by users in special situations. +fn handle_read_limit_change(vars: &EnvStack) { + let read_byte_limit = vars + .get_unless_empty(L!("fish_read_limit")) + .map(|v| v.as_string()) + .and_then(|v| { + // We use fish_wcstoul() to support leading/trailing whitespace + match (crate::wutil::fish_wcstoul(&v).ok()) + // wcstoul() returns a u64 but want a usize. Handle overflow on 32-bit platforms. + .and_then(|_u64| usize::try_from(_u64).ok()) + { + Some(v) => Some(v), + None => { + // We intentionally warn here even in non-interactive mode. + FLOGF!(warning, "Ignoring invalid $fish_read_limit"); + None + } + } + }); + + // Clippy should recognize comments in an empty match branch as a valid pattern! + #[allow(clippy::single_match)] + match read_byte_limit { + Some(new_limit) => READ_BYTE_LIMIT.store(new_limit, Ordering::Relaxed), + None => { + // TODO: reset READ_BYTE_LIMIT to the default value on receiving an invalid value + // instead of persisting the previous value, which may or may not have been the + // default. + } + } +} + +fn handle_fish_trace(vars: &EnvStack) { + let enabled = vars.get_unless_empty(L!("fish_trace")).is_some(); + crate::trace::trace_set_enabled(enabled); +} + +pub fn env_dispatch_init(vars: &EnvStack) { + use once_cell::sync::Lazy; + + run_inits(vars); + // env_dispatch_var_change() purposely supresses change notifications until the dispatch table + // was initialized elsewhere (either explicitly as below or via deref of VAR_DISPATCH_TABLE). + Lazy::force(&VAR_DISPATCH_TABLE); +} + +pub fn env_dispatch_init_ffi() { + let vars = EnvStack::principal(); + env_dispatch_init(vars); +} + +/// Runs the subset of dispatch functions that need to be called at startup. +fn run_inits(vars: &EnvStack) { + init_locale(vars); + init_curses(vars); + guess_emoji_width(vars); + update_wait_on_escape_ms(vars); + handle_read_limit_change(vars); + handle_fish_use_posix_spawn_change(vars); + handle_fish_trace(vars); +} + +/// Updates our idea of whether we support term256 and term24bit (see issue #10222). +fn update_fish_color_support(vars: &EnvStack) { + // Detect or infer term256 support. If fish_term256 is set, we respect it. Otherwise, infer it + // from $TERM or use terminfo. + + let term = vars + .get(L!("TERM")) + .map(|v| v.as_string()) + .unwrap_or_else(WString::new); + let max_colors = curses::term().and_then(|term| term.max_colors); + let mut supports_256color = false; + let mut supports_24bit = false; + + if let Some(fish_term256) = vars.get(L!("fish_term256")).map(|v| v.as_string()) { + // $fish_term256 + supports_256color = crate::wcstringutil::bool_from_string(&fish_term256); + FLOGF!( + term_support, + "256-color support determined by $fish_term256:", + supports_256color + ); + } else if term.find(L!("256color")).is_some() { + // TERM contains "256color": 256 colors explicitly supported. + supports_256color = true; + FLOGF!(term_support, "256-color support enabled for TERM", term); + } else if term.find(L!("xterm")).is_some() { + // Assume that all "xterm" terminals can handle 256 + supports_256color = true; + FLOGF!(term_support, "256-color support enabled for TERM", term); + } + // See if terminfo happens to identify 256 colors + else if let Some(max_colors) = max_colors { + supports_256color = max_colors >= 256; + FLOGF!( + term_support, + "256-color support:", + max_colors, + "per termcap/terminfo entry for", + term + ); + } + + if let Some(fish_term24bit) = vars.get(L!("fish_term24bit")).map(|v| v.as_string()) { + // $fish_term24bit + supports_24bit = crate::wcstringutil::bool_from_string(&fish_term24bit); + FLOGF!( + term_support, + "$fish_term24bit preference: 24-bit color", + if supports_24bit { + "enabled" + } else { + "disabled" + } + ); + } else if vars.get(L!("STY")).is_some() || term.starts_with(L!("eterm")) { + // Screen and emacs' ansi-term swallow true-color sequences, so we ignore them unless + // force-enabled. + supports_24bit = false; + FLOGF!( + term_support, + "True-color support: disabled for eterm/screen" + ); + } else if max_colors.unwrap_or(0) > 32767 { + // $TERM wins, xterm-direct reports 32767 colors and we assume that's the minimum as xterm + // is weird when it comes to color. + supports_24bit = true; + FLOGF!( + term_support, + "True-color support: enabled per termcap/terminfo for", + term, + "with", + max_colors.unwrap(), + "colors" + ); + } else if let Some(ct) = vars.get(L!("COLORTERM")).map(|v| v.as_string()) { + // If someone sets $COLORTERM, that's the sort of color they want. + if ct == "truecolor" || ct == "24bit" { + supports_24bit = true; + } + FLOGF!( + term_support, + "True-color support", + if supports_24bit { + "enabled" + } else { + "disabled" + }, + "per $COLORTERM", + ct + ); + } else if vars.get(L!("KONSOLE_VERSION")).is_some() + || vars.get(L!("KONSOLE_PROFILE_NAME")).is_some() + { + // All Konsole versions that use $KONSOLE_VERSION are new enough to support this, so no + // check is needed. + supports_24bit = true; + FLOGF!(term_support, "True-color support: enabled for Konsole"); + } else if let Some(it) = vars.get(L!("ITERM_SESSION_ID")).map(|v| v.as_string()) { + // Supporting versions of iTerm include a colon here. + // We assume that if this is iTerm it can't also be st, so having this check inside is okay. + if !it.contains(':') { + supports_24bit = true; + FLOGF!(term_support, "True-color support: enabled for iTerm"); + } + } else if term.starts_with("st-") { + supports_24bit = true; + FLOGF!(term_support, "True-color support: enabling for st"); + } else if let Some(vte) = vars.get(L!("VTE_VERSION")).map(|v| v.as_string()) { + if fish_wcstoi(&vte).unwrap_or(0) > 3600 { + supports_24bit = true; + FLOGF!( + term_support, + "True-color support: enabled for VTE version", + vte + ); + } + } + + let mut color_support = ColorSupport::NONE; + color_support.set(ColorSupport::TERM_256COLOR, supports_256color); + color_support.set(ColorSupport::TERM_24BIT, supports_24bit); + crate::output::output_set_color_support(color_support); +} + +/// Try to initialize the terminfo/curses subsystem using our fallback terminal name. Do not set +/// `$TERM` to our fallback. We're only doing this in the hope of getting a functional shell. +/// If we launch an external command that uses `$TERM`, it should get the same value we were given, +/// if any. +fn initialize_curses_using_fallbacks(vars: &EnvStack) { + // xterm-256color is the most used terminal type by a massive margin, especially counting + // terminals that are mostly compatible. + const FALLBACKS: [&str; 4] = ["xterm-256color", "xterm", "ansi", "dumb"]; + + let current_term = vars + .get_unless_empty(L!("TERM")) + .map(|v| v.as_string()) + .unwrap_or(Default::default()); + + for term in FALLBACKS { + // If $TERM is already set to the fallback name we're about to use, there's no point in + // seeing if the fallback name can be used. + if current_term == term { + continue; + } + + // `term` here is one of our hard-coded strings above; we can unwrap because we can + // guarantee it doesn't contain any interior NULs. + let term_cstr = CString::new(term).unwrap(); + let success = curses::setup(Some(&term_cstr), libc::STDOUT_FILENO, |term| { + apply_term_hacks(vars, term) + }) + .is_some(); + if is_interactive_session() { + if success { + FLOGF!(warning, wgettext!("Using fallback terminal type"), term); + } else { + FLOGF!( + warning, + wgettext!("Could not set up terminal using the fallback terminal type"), + term, + ); + } + } + + if success { + break; + } + } +} + +/// Apply any platform- or environment-specific hacks to our curses [`Term`] instance. +fn apply_term_hacks(vars: &EnvStack, term: &mut Term) { + if cfg!(target_os = "macos") { + // Hack in missing italics and dim capabilities omitted from macOS xterm-256color terminfo. + // Improves the user experience under Terminal.app and iTerm. + let term_prog = vars + .get(L!("TERM_PROGRAM")) + .map(|v| v.as_string()) + .unwrap_or(WString::new()); + if term_prog == "Apple_Terminal" || term_prog == "iTerm.app" { + if let Some(term_val) = vars.get(L!("TERM")).map(|v| v.as_string()) { + if term_val == "xterm-256color" { + const SITM_ESC: &[u8] = b"\x1B[3m"; + const RITM_ESC: &[u8] = b"\x1B[23m"; + const DIM_ESC: &[u8] = b"\x1B[2m"; + + if term.enter_italics_mode.is_none() { + term.enter_italics_mode = Some(SITM_ESC.to_cstring()); + } + if term.exit_italics_mode.is_none() { + term.exit_italics_mode = Some(RITM_ESC.to_cstring()); + } + if term.enter_dim_mode.is_none() { + term.enter_dim_mode = Some(DIM_ESC.to_cstring()); + } + } + } + } + } +} + +/// Apply any platform- or environment-specific hacks that don't involve a `Term` instance. +fn apply_non_term_hacks(vars: &EnvStack) { + // Midnight Commander tries to extract the last line of the prompt, and does so in a way that is + // broken if you do '\r' after it like we normally do. + // See https://midnight-commander.org/ticket/4258. + if vars.get(L!("MC_SID")).is_some() { + crate::ffi::screen_set_midnight_commander_hack(); + } +} + +/// This is a pretty lame heuristic for detecting terminals that do not support setting the title. +/// If we recognise the terminal name as that of a virtual terminal, we assume it supports setting +/// the title. If we recognise it as that of a console, we assume it does not support setting the +/// title. Otherwise we check the ttyname and see if we believe it is a virtual terminal. +/// +/// One situation in which this breaks down is with screen, since screen supports setting the +/// terminal title if the underlying terminal does so, but will print garbage on terminals that +/// don't. Since we can't see the underlying terminal below screen there is no way to fix this. +fn does_term_support_setting_title(vars: &EnvStack) -> bool { + #[rustfmt::skip] + const TITLE_TERMS: &[&wstr] = &[ + L!("xterm"), L!("screen"), L!("tmux"), L!("nxterm"), + L!("rxvt"), L!("alacritty"), L!("wezterm"), + ]; + + let Some(term) = vars.get_unless_empty(L!("TERM")).map(|v| v.as_string()) else { + return false; + }; + let term: &wstr = term.as_ref(); + + let recognized = TITLE_TERMS.contains(&term) + || term.starts_with(L!("xterm-")) + || term.starts_with(L!("screen-")) + || term.starts_with(L!("tmux-")); + if !recognized { + if [ + L!("linux"), + L!("dumb"), + L!("vt100"), // NetBSD + L!("wsvt25"), + ] + .contains(&term) + { + return false; + } + + let mut buf = [b'\0'; libc::PATH_MAX as usize]; + let retval = + unsafe { libc::ttyname_r(libc::STDIN_FILENO, buf.as_mut_ptr().cast(), buf.len()) }; + let buf = &buf[..buf.iter().position(|c| *c == b'\0').unwrap()]; + if retval != 0 + || buf.windows(b"tty".len()).any(|w| w == b"tty") + || buf.windows(b"/vc/".len()).any(|w| w == b"/vc/") + { + return false; + } + } + + true +} + +// Initialize the curses subsystem +fn init_curses(vars: &EnvStack) { + for var_name in CURSES_VARIABLES { + if let Some(value) = vars + .getf_unless_empty(var_name, EnvMode::EXPORT) + .map(|v| v.as_string()) + { + FLOGF!(term_support, "curses var", var_name, "=", value); + setenv_lock(var_name, &value, true); + } else { + FLOGF!(term_support, "curses var", var_name, "is missing or empty"); + unsetenv_lock(var_name); + } + } + + if curses::setup(None, libc::STDOUT_FILENO, |term| { + apply_term_hacks(vars, term) + }) + .is_none() + { + if is_interactive_session() { + let term = vars.get_unless_empty(L!("TERM")).map(|v| v.as_string()); + FLOGF!(warning, wgettext!("Could not set up terminal.")); + if let Some(term) = term { + FLOGF!(warning, wgettext!("TERM environment variable set to"), term); + FLOGF!( + warning, + wgettext!("Check that this terminal type is supported on this system.") + ); + } else { + FLOGF!(warning, wgettext!("TERM environment variable not set.")); + } + } + + initialize_curses_using_fallbacks(vars); + } + + // Configure hacks that apply regardless of whether we successfully init curses or not. + apply_non_term_hacks(vars); + + // Store some global variables that reflect the term's capabilities + CAN_SET_TERM_TITLE.store(does_term_support_setting_title(vars), Ordering::Relaxed); + if let Some(term) = curses::term() { + TERM_HAS_XN.store(term.eat_newline_glitch, Ordering::Relaxed); + } + + update_fish_color_support(vars); + // Invalidate the cached escape sequences since they may no longer be valid. + crate::ffi::screen_clear_layout_cache_ffi(); + CURSES_INITIALIZED.store(true, Ordering::Relaxed); +} + +/// Initialize the locale subsystem +fn init_locale(vars: &EnvStack) { + #[rustfmt::skip] + const UTF8_LOCALES: &[&str] = &[ + "C.UTF-8", "en_US.UTF-8", "en_GB.UTF-8", "de_DE.UTF-8", "C.utf8", "UTF-8", + ]; + + let old_msg_locale = unsafe { + let old = libc::setlocale(libc::LC_MESSAGES, std::ptr::null()); + // We have to make a copy because the subsequent setlocale() call to change the locale will + // invalidate the pointer from this setlocale() call. + CStr::from_ptr(old.cast()).to_owned() + }; + + for var_name in LOCALE_VARIABLES { + let var = vars + .getf_unless_empty(var_name, EnvMode::EXPORT) + .map(|v| v.as_string()); + if let Some(value) = var { + FLOGF!(env_locale, "locale var", var_name, "=", value); + setenv_lock(var_name, &value, true); + } else { + FLOGF!(env_locale, "locale var", var_name, "is missing or empty"); + unsetenv_lock(var_name); + } + } + + let locale = unsafe { CStr::from_ptr(libc::setlocale(libc::LC_ALL, b"\0".as_ptr().cast())) }; + + // Try to get a multibyte-capable encoding. + // A "C" locale is broken for our purposes: any wchar function will break on it. So we try + // *really, really, really hard* to not have one. + let fix_locale = vars + .get_unless_empty(L!("fish_allow_singlebyte_locale")) + .map(|v| v.as_string()) + .map(|allow_c| !crate::wcstringutil::bool_from_string(&allow_c)) + .unwrap_or(true); + + if fix_locale && crate::compat::MB_CUR_MAX() == 1 { + FLOGF!(env_locale, "Have singlebyte locale, trying to fix."); + for locale in UTF8_LOCALES { + unsafe { + let locale = CString::new(locale.to_owned()).unwrap(); + libc::setlocale(libc::LC_CTYPE, locale.as_ptr()); + } + if crate::compat::MB_CUR_MAX() > 1 { + FLOGF!(env_locale, "Fixed locale:", locale); + break; + } + } + + if crate::compat::MB_CUR_MAX() == 1 { + FLOGF!(env_locale, "Failed to fix locale."); + } + } + + // We *always* use a C-locale for numbers because we want '.' (except for in printf). + unsafe { + libc::setlocale(libc::LC_NUMERIC, b"C\0".as_ptr().cast()); + } + + // See that we regenerate our special locale for numbers + crate::locale::invalidate_numeric_locale(); + crate::common::fish_setlocale(); + FLOGF!( + env_locale, + "init_locale() setlocale():", + locale.to_string_lossy() + ); + + let new_msg_locale = + unsafe { CStr::from_ptr(libc::setlocale(libc::LC_MESSAGES, std::ptr::null())) }; + FLOGF!( + env_locale, + "Old LC_MESSAGES locale:", + old_msg_locale.to_string_lossy() + ); + FLOGF!( + env_locale, + "New LC_MESSAGES locale:", + new_msg_locale.to_string_lossy() + ); + + #[cfg(feature = "gettext")] + { + if old_msg_locale.as_c_str() != new_msg_locale { + // Make change known to GNU gettext. + extern "C" { + static mut _nl_msg_cat_cntr: libc::c_int; + } + unsafe { + _nl_msg_cat_cntr += 1; + } + } + } +} + +pub fn use_posix_spawn() -> bool { + USE_POSIX_SPAWN.load(Ordering::Relaxed) +} + +/// Whether or not we are running on an OS where we allow ourselves to use `posix_spawn()`. +const fn allow_use_posix_spawn() -> bool { + #![allow(clippy::if_same_then_else)] + #![allow(clippy::needless_bool)] + // OpenBSD's posix_spawn returns status 127 instead of erroring with ENOEXEC when faced with a + // shebang-less script. Disable posix_spawn on OpenBSD. + if cfg!(target_os = "openbsd") { + false + } else if cfg!(not(target_os = "linux")) { + true + } else { + // The C++ code used __GLIBC_PREREQ(2, 24) && !defined(__UCLIBC__) to determine if we'll use + // posix_spawn() by default on Linux. Surprise! We don't have to worry about porting that + // logic here because the libc crate only supports 2.26+ atm. + // See https://github.com/rust-lang/libc/issues/1412 + true + } +} + +/// Returns true if we think the terminal support setting its title. +pub fn term_supports_setting_title() -> bool { + CAN_SET_TERM_TITLE.load(Ordering::Relaxed) +} diff --git a/fish-rust/src/ffi.rs b/fish-rust/src/ffi.rs index 27c0cc47d..94e2eef41 100644 --- a/fish-rust/src/ffi.rs +++ b/fish-rust/src/ffi.rs @@ -20,8 +20,8 @@ pub type wchar_t = u32; include_cpp! { #include "builtin.h" #include "common.h" + #include "complete.h" #include "env.h" - #include "env_dispatch.h" #include "env_universal_common.h" #include "event.h" #include "fallback.h" @@ -30,7 +30,9 @@ include_cpp! { #include "flog.h" #include "function.h" #include "highlight.h" + #include "history.h" #include "io.h" + #include "input_common.h" #include "kill.h" #include "parse_constants.h" #include "parser.h" @@ -38,6 +40,7 @@ include_cpp! { #include "path.h" #include "proc.h" #include "reader.h" + #include "screen.h" #include "tokenizer.h" #include "wildcard.h" #include "wutil.h" @@ -55,7 +58,6 @@ include_cpp! { generate_pod!("pipes_ffi_t") generate!("environment_t") - generate!("env_dispatch_var_change_ffi") generate!("env_stack_t") generate!("env_var_t") generate!("env_universal_t") @@ -134,6 +136,19 @@ include_cpp! { generate!("kill_entries_ffi") generate!("get_history_variable_text_ffi") + + generate!("is_interactive_session") + generate!("set_interactive_session") + generate!("screen_set_midnight_commander_hack") + generate!("screen_clear_layout_cache_ffi") + generate!("reader_schedule_prompt_repaint") + generate!("reader_change_history") + generate!("history_session_id") + generate!("reader_change_cursor_selection_mode") + generate!("reader_set_autosuggestion_enabled_ffi") + generate!("function_invalidate_path") + generate!("complete_invalidate_path") + generate!("update_wait_on_escape_ms_ffi") } impl parser_t { diff --git a/fish-rust/src/lib.rs b/fish-rust/src/lib.rs index 612c113f9..b87f5f7ef 100644 --- a/fish-rust/src/lib.rs +++ b/fish-rust/src/lib.rs @@ -20,6 +20,7 @@ mod color; mod compat; mod curses; mod env; +mod env_dispatch; mod event; mod expand; mod fallback; diff --git a/fish-rust/src/termsize.rs b/fish-rust/src/termsize.rs index fd26e1536..164b6966c 100644 --- a/fish-rust/src/termsize.rs +++ b/fish-rust/src/termsize.rs @@ -1,6 +1,6 @@ // Support for exposing the terminal size. use crate::common::assert_sync; -use crate::env::EnvMode; +use crate::env::{EnvMode, Environment}; use crate::ffi::{environment_t, parser_t, Repin}; use crate::flog::FLOG; use crate::wchar::{WString, L}; @@ -16,9 +16,11 @@ mod termsize_ffi { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct Termsize { /// Width of the terminal, in columns. + // TODO: Change to u32 pub width: isize, /// Height of the terminal, in rows. + // TODO: Change to u32 pub height: isize, } @@ -224,7 +226,34 @@ impl TermsizeContainer { } /// Note that COLUMNS and/or LINES global variables changed. - fn handle_columns_lines_var_change(&self, vars: &environment_t) { + fn handle_columns_lines_var_change(&self, vars: &dyn Environment) { + // Do nothing if we are the ones setting it. + if self.setting_env_vars.load(Ordering::Relaxed) { + return; + } + // Construct a new termsize from COLUMNS and LINES, then set it in our data. + let new_termsize = Termsize { + width: vars + .getf(L!("COLUMNS"), EnvMode::GLOBAL) + .map(|v| v.as_string()) + .and_then(|v| fish_wcstoi(&v).ok().map(|h| h as isize)) + .unwrap_or(Termsize::DEFAULT_WIDTH), + height: vars + .getf(L!("LINES"), EnvMode::GLOBAL) + .map(|v| v.as_string()) + .and_then(|v| fish_wcstoi(&v).ok().map(|h| h as isize)) + .unwrap_or(Termsize::DEFAULT_HEIGHT), + }; + + // Store our termsize as an environment override. + self.data + .lock() + .unwrap() + .mark_override_from_env(new_termsize); + } + + /// Note that COLUMNS and/or LINES global variables changed. + fn handle_columns_lines_var_change_ffi(&self, vars: &environment_t) { // Do nothing if we are the ones setting it. if self.setting_env_vars.load(Ordering::Relaxed) { return; @@ -278,13 +307,16 @@ pub fn termsize_last() -> Termsize { } /// Called when the COLUMNS or LINES variables are changed. -/// The pointer is to an environment_t, but has the wrong type to satisfy cxx. -pub fn handle_columns_lines_var_change_ffi(vars_ptr: *const u8) { - assert!(!vars_ptr.is_null()); - let vars: &environment_t = unsafe { &*(vars_ptr as *const environment_t) }; +pub fn handle_columns_lines_var_change(vars: &dyn Environment) { SHARED_CONTAINER.handle_columns_lines_var_change(vars); } +fn handle_columns_lines_var_change_ffi(vars_ptr: *const u8) { + assert!(!vars_ptr.is_null()); + let vars: &environment_t = unsafe { &*(vars_ptr.cast()) }; + SHARED_CONTAINER.handle_columns_lines_var_change_ffi(vars); +} + /// Called to initialize the termsize. /// The pointer is to an environment_t, but has the wrong type to satisfy cxx. pub fn termsize_initialize_ffi(vars_ptr: *const u8) -> Termsize { @@ -349,13 +381,13 @@ add_test!("test_termsize", || { // Now the tty's termsize doesn't matter. parser.set_var(L!("COLUMNS"), &[L!("75")], env_global); parser.set_var(L!("LINES"), &[L!("150")], env_global); - ts.handle_columns_lines_var_change(parser.get_var_stack_env()); + ts.handle_columns_lines_var_change_ffi(parser.get_var_stack_env()); assert_eq!(ts.last(), Termsize::new(75, 150)); assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "75"); assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "150"); parser.set_var(L!("COLUMNS"), &[L!("33")], env_global); - ts.handle_columns_lines_var_change(parser.get_var_stack_env()); + ts.handle_columns_lines_var_change_ffi(parser.get_var_stack_env()); assert_eq!(ts.last(), Termsize::new(33, 150)); // Oh it got SIGWINCH, now the tty matters again. diff --git a/fish-rust/src/threads.rs b/fish-rust/src/threads.rs index fc096c7df..399e094c1 100644 --- a/fish-rust/src/threads.rs +++ b/fish-rust/src/threads.rs @@ -307,9 +307,8 @@ fn spawn_ffi(callback: &cxx::SharedPtr) -> bool { /// /// This function is always defined but is a no-op if not running under ASAN. This is to make it /// more ergonomic to call it in general and also makes it possible to call it via ffi at all. -pub fn asan_maybe_exit(#[allow(unused)] code: i32) { - #[cfg(feature = "asan")] - { +pub fn asan_maybe_exit(code: i32) { + if cfg!(feature = "asan") { asan_before_exit(); unsafe { libc::exit(code); @@ -323,15 +322,9 @@ pub fn asan_maybe_exit(#[allow(unused)] code: i32) { /// This function is always defined but is a no-op if not running under ASAN. This is to make it /// more ergonomic to call it in general and also makes it possible to call it via ffi at all. pub fn asan_before_exit() { - #[cfg(feature = "asan")] - if !is_forked_child() { - unsafe { - // Free ncurses terminal state - extern "C" { - fn env_cleanup(); - } - env_cleanup(); - } + if cfg!(feature = "asan") && !is_forked_child() { + // Free ncurses terminal state + crate::curses::reset(); } } diff --git a/src/env.cpp b/src/env.cpp index d60e6c928..0fe0b7428 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -20,7 +20,7 @@ #include "abbrs.h" #include "common.h" -#include "env_dispatch.h" +#include "env_dispatch.rs.h" #include "env_universal_common.h" #include "event.h" #include "fallback.h" // IWYU pragma: keep @@ -364,7 +364,7 @@ void env_init(const struct config_paths_t *paths, bool do_uvars, bool default_pa vars.set_one(FISH_BIND_MODE_VAR, ENV_GLOBAL, DEFAULT_BIND_MODE); // Allow changes to variables to produce events. - env_dispatch_init(vars); + env_dispatch_init_ffi(/* vars */); init_input(); diff --git a/src/env.h b/src/env.h index 5cdda13d8..404e6f280 100644 --- a/src/env.h +++ b/src/env.h @@ -308,11 +308,6 @@ class env_stack_t final : public environment_t { rust::Box impl_; }; -bool get_use_posix_spawn(); - -/// Returns true if we think the terminal supports setting its title. -bool term_supports_setting_title(); - #if INCLUDE_RUST_HEADERS struct EnvDyn; /// Wrapper around rust's `&dyn Environment` deriving from `environment_t`. diff --git a/src/env_dispatch.h b/src/env_dispatch.h deleted file mode 100644 index f01ee6534..000000000 --- a/src/env_dispatch.h +++ /dev/null @@ -1,22 +0,0 @@ -// Prototypes for functions that react to environment variable changes -#ifndef FISH_ENV_DISPATCH_H -#define FISH_ENV_DISPATCH_H - -#include "config.h" // IWYU pragma: keep - -#include "common.h" - -class environment_t; -class env_stack_t; - -/// Initialize variable dispatch. -void env_dispatch_init(const environment_t &vars); - -/// React to changes in variables like LANG which require running some code. -void env_dispatch_var_change(const wcstring &key, env_stack_t &vars); - -/// FFI wrapper which always uses the principal stack. -/// TODO: pass in the variables directly. -void env_dispatch_var_change_ffi(const wcstring &key /*, env_stack_t &vars */); - -#endif diff --git a/src/exec.cpp b/src/exec.cpp index f69e2fb12..e66b0d190 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -32,6 +32,7 @@ #include "builtin.h" #include "common.h" #include "env.h" +#include "env_dispatch.rs.h" #include "exec.h" #include "fallback.h" // IWYU pragma: keep #include "fds.h" @@ -214,7 +215,7 @@ bool is_thompson_shell_script(const char *path) { static bool can_use_posix_spawn_for_job(const std::shared_ptr &job, const dup2_list_t &dup2s) { // Is it globally disabled? - if (!get_use_posix_spawn()) return false; + if (!use_posix_spawn()) return false; // Hack - do not use posix_spawn if there are self-fd redirections. // For example if you were to write: diff --git a/src/history.cpp b/src/history.cpp index 7a0af2aa7..931d614ca 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -1273,10 +1273,10 @@ void history_impl_t::incorporate_external_changes() { } /// Return the prefix for the files to be used for command and read history. -wcstring history_session_id(const environment_t &vars) { +wcstring history_session_id(std::unique_ptr fish_history) { wcstring result = DFLT_FISH_HISTORY_SESSION_ID; - const auto var = vars.get(L"fish_history"); + const auto var = std::move(fish_history); if (var) { wcstring session_id = var->as_string(); if (session_id.empty()) { @@ -1294,6 +1294,13 @@ wcstring history_session_id(const environment_t &vars) { return result; } +wcstring history_session_id(const environment_t &vars) { + auto fish_history = vars.get(L"fish_history"); + auto var = + fish_history ? std::make_unique(*fish_history) : std::unique_ptr{}; + return history_session_id(std::move(var)); +} + path_list_t expand_and_detect_paths(const path_list_t &paths, const environment_t &vars) { ASSERT_IS_BACKGROUND_THREAD(); std::vector result; diff --git a/src/history.h b/src/history.h index fff284321..1d6d49d54 100644 --- a/src/history.h +++ b/src/history.h @@ -316,8 +316,14 @@ class history_search_t { /** Saves the new history to disk. */ void history_save_all(); +#if INCLUDE_RUST_HEADERS /** Return the prefix for the files to be used for command and read history. */ wcstring history_session_id(const environment_t &vars); +#endif + +/** FFI version of above **/ +class env_var_t; +wcstring history_session_id(std::unique_ptr fish_history); /** Given a list of proposed paths and a context, perform variable and home directory expansion, diff --git a/src/input_common.cpp b/src/input_common.cpp index e827b3d70..1f9b4db2d 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -140,6 +140,23 @@ void update_wait_on_escape_ms(const environment_t& vars) { } } +void update_wait_on_escape_ms_ffi(std::unique_ptr fish_escape_delay_ms) { + if (!fish_escape_delay_ms) { + wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT; + return; + } + + long tmp = fish_wcstol(fish_escape_delay_ms->as_string().c_str()); + if (errno || tmp < 10 || tmp >= 5000) { + std::fwprintf(stderr, + L"ignoring fish_escape_delay_ms: value '%ls' " + L"is not an integer or is < 10 or >= 5000 ms\n", + fish_escape_delay_ms->as_string().c_str()); + } else { + wait_on_escape_ms = static_cast(tmp); + } +} + maybe_t input_event_queue_t::try_pop() { if (queue_.empty()) { return none(); diff --git a/src/input_common.h b/src/input_common.h index a53c46b5d..976eb1d16 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -186,6 +186,7 @@ class char_event_t { /// Adjust the escape timeout. class environment_t; void update_wait_on_escape_ms(const environment_t &vars); +void update_wait_on_escape_ms_ffi(std::unique_ptr fish_escape_delay_ms); /// A class which knows how to produce a stream of input events. /// This is a base class; you may subclass it for its override points. diff --git a/src/reader.cpp b/src/reader.cpp index 436acff89..e1ca59268 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -50,6 +50,7 @@ #include "common.h" #include "complete.h" #include "env.h" +#include "env_dispatch.rs.h" #include "event.h" #include "exec.h" #include "expand.h" diff --git a/src/reader.h b/src/reader.h index 9adc8467d..56801d579 100644 --- a/src/reader.h +++ b/src/reader.h @@ -174,10 +174,10 @@ void reader_change_cursor_selection_mode(cursor_selection_mode_t selection_mode) void reader_change_cursor_selection_mode(uint8_t selection_mode); #endif +struct EnvDyn; /// Enable or disable autosuggestions based on the associated variable. void reader_set_autosuggestion_enabled(const env_stack_t &vars); - -void reader_set_autosuggestion_enabled_ffi(bool); +void reader_set_autosuggestion_enabled_ffi(bool enabled); /// Write the title to the titlebar. This function is called just before a new application starts /// executing and just after it finishes.