Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-28 20:55:11 +00:00
Code Issues Projects Releases Packages Wiki Activity

Expose Rust EnvStack from parser_t

Browse source 
Prior to this change, parser_t exposed an environment_t, and Rust had to go
through that. But because we have implemented Environment in Rust, it is
better to just expose the native Environment from parser_t. Make that
change and update call sites.
This commit is contained in:
ridiculousfish 2023-06-19 12:03:46 -07:00
parent 6936c944c1
commit f77dc2451e
8 changed files with 73 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
fish-rust/src/builtins/argparse.rs
View file

@ -6,8 +6,7 @@ use crate::builtins::shared::{
BUILTIN_ERR_MAX_ARG_COUNT1, BUILTIN_ERR_MIN_ARG_COUNT1, BUILTIN_ERR_UNKNOWN, STATUS_CMD_ERROR, BUILTIN_ERR_MAX_ARG_COUNT1, BUILTIN_ERR_MIN_ARG_COUNT1, BUILTIN_ERR_UNKNOWN, STATUS_CMD_ERROR,
STATUS_CMD_OK, STATUS_INVALID_ARGS, STATUS_CMD_OK, STATUS_INVALID_ARGS,
}; };
use crate::env::EnvMode; use crate::env::{EnvMode, EnvStack};
use crate::ffi::env_stack_t;
use crate::ffi::parser_t; use crate::ffi::parser_t;
use crate::ffi::Repin; use crate::ffi::Repin;
use crate::wchar::{wstr, WString, L}; use crate::wchar::{wstr, WString, L};
@ -649,25 +648,25 @@ fn validate_arg<'opts>(
return STATUS_CMD_OK; return STATUS_CMD_OK;
} }
// TODO: in C++ this set directly in the vars (so does not emit events), reimplement that. let vars = parser.get_vars();
parser.get_var_stack().pin().push(true); vars.push(true /* new_scope */);
let env_mode = EnvMode::LOCAL | EnvMode::EXPORT; let env_mode = EnvMode::LOCAL | EnvMode::EXPORT;
parser.set_var(L!("_argparse_cmd"), &[&opts_name], env_mode); vars.set_one(L!("_argparse_cmd"), env_mode, opts_name.to_owned());
let flag_name = WString::from(VAR_NAME_PREFIX) + "name"; let flag_name = WString::from(VAR_NAME_PREFIX) + "name";
if is_long_flag { if is_long_flag {
parser.set_var(flag_name, &[&opt_spec.long_flag], env_mode); vars.set_one(&flag_name, env_mode, opt_spec.long_flag.to_owned());
} else { } else {
parser.set_var( vars.set_one(
flag_name, &flag_name,
&[&wstr::from_char_slice(&[opt_spec.short_flag])],
env_mode, env_mode,
WString::from_chars(vec![opt_spec.short_flag]),
); );
} }
parser.set_var( vars.set_one(
WString::from(VAR_NAME_PREFIX) + "value", &(WString::from(VAR_NAME_PREFIX) + "value"),
&[&woptarg],
env_mode, env_mode,
woptarg.to_owned(),
); );
let mut cmd_output = Vec::new(); let mut cmd_output = Vec::new();
@ -678,7 +677,7 @@ fn validate_arg<'opts>(
streams.err.append(output); streams.err.append(output);
streams.err.append1('\n'); streams.err.append1('\n');
} }
parser.get_var_stack().pin().pop(); vars.pop();
return retval; return retval;
} }
@ -922,7 +921,7 @@ fn check_min_max_args_constraints(
} }
/// Put the result of parsing the supplied args into the caller environment as local vars. /// Put the result of parsing the supplied args into the caller environment as local vars.
fn set_argparse_result_vars(vars: &mut env_stack_t, opts: &ArgParseCmdOpts) { fn set_argparse_result_vars(vars: &EnvStack, opts: &ArgParseCmdOpts) {
for opt_spec in opts.options.values() { for opt_spec in opts.options.values() {
if opt_spec.num_seen == 0 { if opt_spec.num_seen == 0 {
continue; continue;
@ -931,7 +930,7 @@ fn set_argparse_result_vars(vars: &mut env_stack_t, opts: &ArgParseCmdOpts) {
if opt_spec.short_flag_valid { if opt_spec.short_flag_valid {
let mut var_name = WString::from(VAR_NAME_PREFIX); let mut var_name = WString::from(VAR_NAME_PREFIX);
var_name.push(opt_spec.short_flag); var_name.push(opt_spec.short_flag);
vars.set_var(&var_name, opt_spec.vals.as_slice(), EnvMode::LOCAL); vars.set(&var_name, EnvMode::LOCAL, opt_spec.vals.clone());
} }
if !opt_spec.long_flag.is_empty() { if !opt_spec.long_flag.is_empty() {
@ -942,11 +941,12 @@ fn set_argparse_result_vars(vars: &mut env_stack_t, opts: &ArgParseCmdOpts) {
.chars() .chars()
.map(|c| if fish_iswalnum(c) { c } else { '_' }); .map(|c| if fish_iswalnum(c) { c } else { '_' });
let var_name_long: WString = VAR_NAME_PREFIX.chars().chain(long_flag).collect(); let var_name_long: WString = VAR_NAME_PREFIX.chars().chain(long_flag).collect();
vars.set_var(var_name_long, opt_spec.vals.as_slice(), EnvMode::LOCAL); vars.set(&var_name_long, EnvMode::LOCAL, opt_spec.vals.clone());
} }
} }
vars.set_var(L!("argv"), opts.args.as_slice(), EnvMode::LOCAL); let args = opts.args.iter().map(|&s| s.to_owned()).collect();
vars.set(L!("argv"), EnvMode::LOCAL, args);
} }
/// The argparse builtin. This is explicitly not compatible with the BSD or GNU version of this /// The argparse builtin. This is explicitly not compatible with the BSD or GNU version of this
@ -1004,7 +1004,7 @@ pub fn argparse(
return retval; return retval;
} }
set_argparse_result_vars(parser.get_var_stack(), &opts); set_argparse_result_vars(&parser.get_vars(), &opts);
return retval; return retval;
} }

2
fish-rust/src/env/env_ffi.rs vendored
View file

@ -220,7 +220,7 @@ impl EnvDynFFI {
} }
/// FFI wrapper around EnvStackRef. /// FFI wrapper around EnvStackRef.
pub struct EnvStackRefFFI(EnvStackRef); pub struct EnvStackRefFFI(pub EnvStackRef);
impl EnvStackRefFFI { impl EnvStackRefFFI {
fn getf(&self, name: &CxxWString, mode: u16) -> *mut EnvVar { fn getf(&self, name: &CxxWString, mode: u16) -> *mut EnvVar {

2
fish-rust/src/env/mod.rs vendored
View file

@ -4,7 +4,7 @@ mod environment_impl;
pub mod var; pub mod var;
use crate::common::ToCString; use crate::common::ToCString;
pub use env_ffi::EnvStackSetResult; pub use env_ffi::{EnvStackRefFFI, EnvStackSetResult};
pub use environment::*; pub use environment::*;
use std::sync::atomic::{AtomicBool, AtomicUsize}; use std::sync::atomic::{AtomicBool, AtomicUsize};
pub use var::*; pub use var::*;

36
fish-rust/src/ffi.rs
View file

@ -4,7 +4,7 @@ use crate::wchar_ffi::WCharToFFI;
use ::std::pin::Pin; use ::std::pin::Pin;
#[rustfmt::skip] #[rustfmt::skip]
use ::std::slice; use ::std::slice;
use crate::env::EnvMode; use crate::env::{EnvMode, EnvStackRef, EnvStackRefFFI};
pub use crate::wait_handle::{ pub use crate::wait_handle::{
WaitHandleRef, WaitHandleRefFFI, WaitHandleStore, WaitHandleStoreFFI, WaitHandleRef, WaitHandleRefFFI, WaitHandleStore, WaitHandleStoreFFI,
}; };
@ -193,26 +193,8 @@ impl parser_t {
unsafe { job.as_ref() } unsafe { job.as_ref() }
} }
/// Helper to get a variable as a string, using the default flags. pub fn get_vars(&mut self) -> EnvStackRef {
pub fn var_as_string(&mut self, name: &wstr) -> Option<WString> { self.pin().vars().from_ffi()
self.pin().vars().unpin().get_as_string(name)
}
pub fn get_var_stack(&mut self) -> &mut env_stack_t {
self.pin().vars().unpin()
}
pub fn get_var_stack_env(&mut self) -> &environment_t {
self.vars_env_ffi()
}
pub fn set_var<T: AsRef<wstr>, U: AsRef<wstr>>(
&mut self,
name: T,
value: &[U],
flags: EnvMode,
) -> libc::c_int {
self.get_var_stack().set_var(name, value, flags)
} }
pub fn get_func_name(&mut self, level: i32) -> Option<WString> { pub fn get_func_name(&mut self, level: i32) -> Option<WString> {
@ -225,6 +207,18 @@ impl parser_t {
unsafe impl Send for env_universal_t {} unsafe impl Send for env_universal_t {}
impl env_stack_t {
/// Access the underlying Rust environment stack.
#[allow(clippy::borrowed_box)]
pub fn from_ffi(&self) -> EnvStackRef {
// Safety: get_impl_ffi returns a pointer to a Box<EnvStackRefFFI>.
let envref = self.get_impl_ffi();
assert!(!envref.is_null());
let env: &Box<EnvStackRefFFI> = unsafe { &*(envref.cast()) };
env.0.clone()
}
}
impl environment_t { impl environment_t {
/// Helper to get a variable as a string, using the default flags. /// Helper to get a variable as a string, using the default flags.
pub fn get_as_string(&self, name: &wstr) -> Option<WString> { pub fn get_as_string(&self, name: &wstr) -> Option<WString> {

69
fish-rust/src/termsize.rs
View file

@ -1,7 +1,7 @@
// Support for exposing the terminal size. // Support for exposing the terminal size.
use crate::common::assert_sync; use crate::common::assert_sync;
use crate::env::{EnvMode, Environment}; use crate::env::{EnvMode, EnvStackRefFFI, EnvVar, Environment};
use crate::ffi::{environment_t, parser_t, Repin}; use crate::ffi::{parser_t, Repin};
use crate::flog::FLOG; use crate::flog::FLOG;
use crate::wchar::{WString, L}; use crate::wchar::{WString, L};
use crate::wchar_ext::ToWString; use crate::wchar_ext::ToWString;
@ -29,7 +29,6 @@ mod termsize_ffi {
pub fn termsize_last() -> Termsize; pub fn termsize_last() -> Termsize;
pub fn termsize_initialize_ffi(vars: *const u8) -> Termsize; pub fn termsize_initialize_ffi(vars: *const u8) -> Termsize;
pub fn termsize_invalidate_tty(); pub fn termsize_invalidate_tty();
pub fn handle_columns_lines_var_change_ffi(vars: *const u8);
pub fn termsize_update_ffi(parser: *mut u8) -> Termsize; pub fn termsize_update_ffi(parser: *mut u8) -> Termsize;
pub fn termsize_handle_winch(); pub fn termsize_handle_winch();
} }
@ -41,10 +40,10 @@ static TTY_TERMSIZE_GEN_COUNT: AtomicU32 = AtomicU32::new(0);
/// Convert an environment variable to an int, or return a default value. /// Convert an environment variable to an int, or return a default value.
/// The int must be >0 and <USHRT_MAX (from struct winsize). /// The int must be >0 and <USHRT_MAX (from struct winsize).
fn var_to_int_or(var: Option<WString>, default: isize) -> isize { fn var_to_int_or(var: Option<EnvVar>, default: isize) -> isize {
if var.is_some() && !var.as_ref().unwrap().is_empty() { let val: WString = var.map(|v| v.as_string()).unwrap_or_default();
#[allow(clippy::unnecessary_unwrap)] if !val.is_empty() {
if let Ok(proposed) = fish_wcstoi(&var.unwrap()) { if let Ok(proposed) = fish_wcstoi(&val) {
if proposed > 0 && proposed <= u16::MAX as i32 { if proposed > 0 && proposed <= u16::MAX as i32 {
return proposed as isize; return proposed as isize;
} }
@ -161,10 +160,10 @@ impl TermsizeContainer {
/// Initialize our termsize, using the given environment stack. /// Initialize our termsize, using the given environment stack.
/// This will prefer to use COLUMNS and LINES, but will fall back to the tty size reader. /// This will prefer to use COLUMNS and LINES, but will fall back to the tty size reader.
/// This does not change any variables in the environment. /// This does not change any variables in the environment.
pub fn initialize(&self, vars: &environment_t) -> Termsize { pub fn initialize(&self, vars: &dyn Environment) -> Termsize {
let new_termsize = Termsize { let new_termsize = Termsize {
width: var_to_int_or(vars.get_as_string_flags(L!("COLUMNS"), EnvMode::GLOBAL), -1), width: var_to_int_or(vars.getf(L!("COLUMNS"), EnvMode::GLOBAL), -1),
height: var_to_int_or(vars.get_as_string_flags(L!("LINES"), EnvMode::GLOBAL), -1), height: var_to_int_or(vars.getf(L!("LINES"), EnvMode::GLOBAL), -1),
}; };
let mut data = self.data.lock().unwrap(); let mut data = self.data.lock().unwrap();
@ -253,7 +252,7 @@ impl TermsizeContainer {
} }
/// Note that COLUMNS and/or LINES global variables changed. /// Note that COLUMNS and/or LINES global variables changed.
fn handle_columns_lines_var_change_ffi(&self, vars: &environment_t) { fn handle_columns_lines_var_change_ffi(&self, vars: &dyn Environment) {
// Do nothing if we are the ones setting it. // Do nothing if we are the ones setting it.
if self.setting_env_vars.load(Ordering::Relaxed) { if self.setting_env_vars.load(Ordering::Relaxed) {
return; return;
@ -261,11 +260,11 @@ impl TermsizeContainer {
// Construct a new termsize from COLUMNS and LINES, then set it in our data. // Construct a new termsize from COLUMNS and LINES, then set it in our data.
let new_termsize = Termsize { let new_termsize = Termsize {
width: var_to_int_or( width: var_to_int_or(
vars.get_as_string_flags(L!("COLUMNS"), EnvMode::GLOBAL), vars.getf(L!("COLUMNS"), EnvMode::GLOBAL),
Termsize::DEFAULT_WIDTH, Termsize::DEFAULT_WIDTH,
), ),
height: var_to_int_or( height: var_to_int_or(
vars.get_as_string_flags(L!("LINES"), EnvMode::GLOBAL), vars.getf(L!("LINES"), EnvMode::GLOBAL),
Termsize::DEFAULT_HEIGHT, Termsize::DEFAULT_HEIGHT,
), ),
}; };
@ -311,18 +310,13 @@ pub fn handle_columns_lines_var_change(vars: &dyn Environment) {
SHARED_CONTAINER.handle_columns_lines_var_change(vars); 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. /// Called to initialize the termsize.
/// The pointer is to an environment_t, but has the wrong type to satisfy cxx. /// The pointer is to a Box<EnvStackRefFFI>, but has the wrong type to satisfy cxx.
#[allow(clippy::borrowed_box)]
pub fn termsize_initialize_ffi(vars_ptr: *const u8) -> Termsize { pub fn termsize_initialize_ffi(vars_ptr: *const u8) -> Termsize {
assert!(!vars_ptr.is_null()); assert!(!vars_ptr.is_null());
let vars: &environment_t = unsafe { &*(vars_ptr as *const environment_t) }; let vars: &Box<EnvStackRefFFI> = unsafe { &*(vars_ptr.cast()) };
SHARED_CONTAINER.initialize(vars) SHARED_CONTAINER.initialize(&*vars.0)
} }
/// Called to update termsize. /// Called to update termsize.
@ -345,6 +339,7 @@ use crate::ffi_tests::add_test;
add_test!("test_termsize", || { add_test!("test_termsize", || {
let env_global = EnvMode::GLOBAL; let env_global = EnvMode::GLOBAL;
let parser: &mut parser_t = unsafe { &mut *parser_t::principal_parser_ffi() }; let parser: &mut parser_t = unsafe { &mut *parser_t::principal_parser_ffi() };
let vars = parser.get_vars();
// Use a static variable so we can pretend we're the kernel exposing a terminal size. // Use a static variable so we can pretend we're the kernel exposing a terminal size.
static STUBBY_TERMSIZE: Mutex<Option<Termsize>> = Mutex::new(None); static STUBBY_TERMSIZE: Mutex<Option<Termsize>> = Mutex::new(None);
@ -374,33 +369,33 @@ add_test!("test_termsize", || {
// Ok now we tell it to update. // Ok now we tell it to update.
ts.updating(parser); ts.updating(parser);
assert_eq!(ts.last(), Termsize::new(42, 84)); assert_eq!(ts.last(), Termsize::new(42, 84));
assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "42"); assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "42");
assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "84"); assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "84");
// Wow someone set COLUMNS and LINES to a weird value. // Wow someone set COLUMNS and LINES to a weird value.
// Now the tty's termsize doesn't matter. // Now the tty's termsize doesn't matter.
parser.set_var(L!("COLUMNS"), &[L!("75")], env_global); vars.set_one(L!("COLUMNS"), env_global, L!("75").to_owned());
parser.set_var(L!("LINES"), &[L!("150")], env_global); vars.set_one(L!("LINES"), env_global, L!("150").to_owned());
ts.handle_columns_lines_var_change_ffi(parser.get_var_stack_env()); ts.handle_columns_lines_var_change(&*parser.get_vars());
assert_eq!(ts.last(), Termsize::new(75, 150)); assert_eq!(ts.last(), Termsize::new(75, 150));
assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "75"); assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "75");
assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "150"); assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "150");
parser.set_var(L!("COLUMNS"), &[L!("33")], env_global); vars.set_one(L!("COLUMNS"), env_global, L!("33").to_owned());
ts.handle_columns_lines_var_change_ffi(parser.get_var_stack_env()); ts.handle_columns_lines_var_change(&*parser.get_vars());
assert_eq!(ts.last(), Termsize::new(33, 150)); assert_eq!(ts.last(), Termsize::new(33, 150));
// Oh it got SIGWINCH, now the tty matters again. // Oh it got SIGWINCH, now the tty matters again.
TermsizeContainer::handle_winch(); TermsizeContainer::handle_winch();
assert_eq!(ts.last(), Termsize::new(33, 150)); assert_eq!(ts.last(), Termsize::new(33, 150));
assert_eq!(ts.updating(parser), stubby_termsize().unwrap()); assert_eq!(ts.updating(parser), stubby_termsize().unwrap());
assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "42"); assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "42");
assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "84"); assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "84");
// Test initialize(). // Test initialize().
parser.set_var(L!("COLUMNS"), &[L!("83")], env_global); vars.set_one(L!("COLUMNS"), env_global, L!("83").to_owned());
parser.set_var(L!("LINES"), &[L!("38")], env_global); vars.set_one(L!("LINES"), env_global, L!("38").to_owned());
ts.initialize(parser.get_var_stack_env()); ts.initialize(&*vars);
assert_eq!(ts.last(), Termsize::new(83, 38)); assert_eq!(ts.last(), Termsize::new(83, 38));
// initialize() even beats the tty reader until a sigwinch. // initialize() even beats the tty reader until a sigwinch.
@ -409,7 +404,7 @@ add_test!("test_termsize", || {
setting_env_vars: AtomicBool::new(false), setting_env_vars: AtomicBool::new(false),
tty_size_reader: stubby_termsize, tty_size_reader: stubby_termsize,
}; };
ts.initialize(parser.get_var_stack_env()); ts.initialize(&*parser.get_vars());
ts2.updating(parser); ts2.updating(parser);
assert_eq!(ts.last(), Termsize::new(83, 38)); assert_eq!(ts.last(), Termsize::new(83, 38));
TermsizeContainer::handle_winch(); TermsizeContainer::handle_winch();

4
src/env.cpp
View file

@ -353,8 +353,8 @@ void env_init(const struct config_paths_t *paths, bool do_uvars, bool default_pa
} }
// Initialize termsize variables. // Initialize termsize variables.
environment_t &env_vars = vars; auto termsize =
auto termsize = termsize_initialize_ffi(reinterpret_cast<const unsigned char *>(&env_vars)); termsize_initialize_ffi(reinterpret_cast<const unsigned char *>(vars.get_impl_ffi()));
if (!vars.get_unless_empty(L"COLUMNS")) if (!vars.get_unless_empty(L"COLUMNS"))
vars.set_one(L"COLUMNS", ENV_GLOBAL, to_string(termsize.width)); vars.set_one(L"COLUMNS", ENV_GLOBAL, to_string(termsize.width));
if (!vars.get_unless_empty(L"LINES")) if (!vars.get_unless_empty(L"LINES"))

4
src/env.h
View file

@ -301,6 +301,10 @@ class env_stack_t final : public environment_t {
// Do not push or pop from this. // Do not push or pop from this.
static env_stack_t &globals(); static env_stack_t &globals();
/// Access the underlying Rust implementation.
/// This returns a const rust::Box<EnvStackRef> *, or in Rust terms, a *const Box<EnvStackRef>.
const void *get_impl_ffi() const { return &impl_; }
private: private:
env_stack_t(rust::Box<EnvStackRef> imp); env_stack_t(rust::Box<EnvStackRef> imp);

3
src/parser.h
View file

@ -393,9 +393,6 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
env_stack_t &vars() { return *variables; } env_stack_t &vars() { return *variables; }
const env_stack_t &vars() const { return *variables; } const env_stack_t &vars() const { return *variables; }
/// Rust helper - variables as an environment_t.
const environment_t &vars_env_ffi() const { return *variables; }
int remove_var_ffi(const wcstring &key, int mode) { return vars().remove(key, mode); } int remove_var_ffi(const wcstring &key, int mode) { return vars().remove(key, mode); }
/// Get the library data. /// Get the library data.