github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-26 19:03:38 +08:00
Code Issues Actions 2 Packages Projects Releases 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,
STATUS_CMD_OK, STATUS_INVALID_ARGS,
};
use crate::env::EnvMode;
use crate::ffi::env_stack_t;
use crate::env::{EnvMode, EnvStack};
use crate::ffi::parser_t;
use crate::ffi::Repin;
use crate::wchar::{wstr, WString, L};
@ -649,25 +648,25 @@ fn validate_arg<'opts>(
return STATUS_CMD_OK;
}
// TODO: in C++ this set directly in the vars (so does not emit events), reimplement that.
parser.get_var_stack().pin().push(true);
let vars = parser.get_vars();
vars.push(true /* new_scope */);
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";
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 {
parser.set_var(
flag_name,
&[&wstr::from_char_slice(&[opt_spec.short_flag])],
vars.set_one(
&flag_name,
env_mode,
WString::from_chars(vec![opt_spec.short_flag]),
);
}
parser.set_var(
WString::from(VAR_NAME_PREFIX) + "value",
&[&woptarg],
vars.set_one(
&(WString::from(VAR_NAME_PREFIX) + "value"),
env_mode,
woptarg.to_owned(),
);
let mut cmd_output = Vec::new();
@ -678,7 +677,7 @@ fn validate_arg<'opts>(
streams.err.append(output);
streams.err.append1('\n');
}
parser.get_var_stack().pin().pop();
vars.pop();
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.
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() {
if opt_spec.num_seen == 0 {
continue;
@ -931,7 +930,7 @@ fn set_argparse_result_vars(vars: &mut env_stack_t, opts: &ArgParseCmdOpts) {
if opt_spec.short_flag_valid {
let mut var_name = WString::from(VAR_NAME_PREFIX);
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() {
@ -942,11 +941,12 @@ fn set_argparse_result_vars(vars: &mut env_stack_t, opts: &ArgParseCmdOpts) {
.chars()
.map(|c| if fish_iswalnum(c) { c } else { '_' });
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
@ -1004,7 +1004,7 @@ pub fn argparse(
return retval;
}
set_argparse_result_vars(parser.get_var_stack(), &opts);
set_argparse_result_vars(&parser.get_vars(), &opts);
return retval;
}

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

@ -220,7 +220,7 @@ impl EnvDynFFI {
}
/// FFI wrapper around EnvStackRef.
pub struct EnvStackRefFFI(EnvStackRef);
pub struct EnvStackRefFFI(pub EnvStackRef);
impl EnvStackRefFFI {
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;
use crate::common::ToCString;
pub use env_ffi::EnvStackSetResult;
pub use env_ffi::{EnvStackRefFFI, EnvStackSetResult};
pub use environment::*;
use std::sync::atomic::{AtomicBool, AtomicUsize};
pub use var::*;

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

@ -4,7 +4,7 @@ use crate::wchar_ffi::WCharToFFI;
use ::std::pin::Pin;
#[rustfmt::skip]
use ::std::slice;
use crate::env::EnvMode;
use crate::env::{EnvMode, EnvStackRef, EnvStackRefFFI};
pub use crate::wait_handle::{
WaitHandleRef, WaitHandleRefFFI, WaitHandleStore, WaitHandleStoreFFI,
};
@ -193,26 +193,8 @@ impl parser_t {
unsafe { job.as_ref() }
}
/// Helper to get a variable as a string, using the default flags.
pub fn var_as_string(&mut self, name: &wstr) -> Option<WString> {
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_vars(&mut self) -> EnvStackRef {
self.pin().vars().from_ffi()
}
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 {}
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 {
/// Helper to get a variable as a string, using the default flags.
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.
use crate::common::assert_sync;
use crate::env::{EnvMode, Environment};
use crate::ffi::{environment_t, parser_t, Repin};
use crate::env::{EnvMode, EnvStackRefFFI, EnvVar, Environment};
use crate::ffi::{parser_t, Repin};
use crate::flog::FLOG;
use crate::wchar::{WString, L};
use crate::wchar_ext::ToWString;
@ -29,7 +29,6 @@ mod termsize_ffi {
pub fn termsize_last() -> Termsize;
pub fn termsize_initialize_ffi(vars: *const u8) -> Termsize;
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_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.
/// The int must be >0 and <USHRT_MAX (from struct winsize).
fn var_to_int_or(var: Option<WString>, default: isize) -> isize {
if var.is_some() && !var.as_ref().unwrap().is_empty() {
#[allow(clippy::unnecessary_unwrap)]
if let Ok(proposed) = fish_wcstoi(&var.unwrap()) {
fn var_to_int_or(var: Option<EnvVar>, default: isize) -> isize {
let val: WString = var.map(|v| v.as_string()).unwrap_or_default();
if !val.is_empty() {
if let Ok(proposed) = fish_wcstoi(&val) {
if proposed > 0 && proposed <= u16::MAX as i32 {
return proposed as isize;
}
@ -161,10 +160,10 @@ impl TermsizeContainer {
/// 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 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 {
width: var_to_int_or(vars.get_as_string_flags(L!("COLUMNS"), EnvMode::GLOBAL), -1),
height: var_to_int_or(vars.get_as_string_flags(L!("LINES"), EnvMode::GLOBAL), -1),
width: var_to_int_or(vars.getf(L!("COLUMNS"), EnvMode::GLOBAL), -1),
height: var_to_int_or(vars.getf(L!("LINES"), EnvMode::GLOBAL), -1),
};
let mut data = self.data.lock().unwrap();
@ -253,7 +252,7 @@ impl TermsizeContainer {
}
/// 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.
if self.setting_env_vars.load(Ordering::Relaxed) {
return;
@ -261,11 +260,11 @@ impl TermsizeContainer {
// Construct a new termsize from COLUMNS and LINES, then set it in our data.
let new_termsize = Termsize {
width: var_to_int_or(
vars.get_as_string_flags(L!("COLUMNS"), EnvMode::GLOBAL),
vars.getf(L!("COLUMNS"), EnvMode::GLOBAL),
Termsize::DEFAULT_WIDTH,
),
height: var_to_int_or(
vars.get_as_string_flags(L!("LINES"), EnvMode::GLOBAL),
vars.getf(L!("LINES"), EnvMode::GLOBAL),
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);
}
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.
/// 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 {
assert!(!vars_ptr.is_null());
let vars: &environment_t = unsafe { &*(vars_ptr as *const environment_t) };
SHARED_CONTAINER.initialize(vars)
let vars: &Box<EnvStackRefFFI> = unsafe { &*(vars_ptr.cast()) };
SHARED_CONTAINER.initialize(&*vars.0)
}
/// Called to update termsize.
@ -345,6 +339,7 @@ use crate::ffi_tests::add_test;
add_test!("test_termsize", || {
let env_global = EnvMode::GLOBAL;
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.
static STUBBY_TERMSIZE: Mutex<Option<Termsize>> = Mutex::new(None);
@ -374,33 +369,33 @@ add_test!("test_termsize", || {
// Ok now we tell it to update.
ts.updating(parser);
assert_eq!(ts.last(), Termsize::new(42, 84));
assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "42");
assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "84");
assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "42");
assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "84");
// Wow someone set COLUMNS and LINES to a weird value.
// 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_ffi(parser.get_var_stack_env());
vars.set_one(L!("COLUMNS"), env_global, L!("75").to_owned());
vars.set_one(L!("LINES"), env_global, L!("150").to_owned());
ts.handle_columns_lines_var_change(&*parser.get_vars());
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");
assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "75");
assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "150");
parser.set_var(L!("COLUMNS"), &[L!("33")], env_global);
ts.handle_columns_lines_var_change_ffi(parser.get_var_stack_env());
vars.set_one(L!("COLUMNS"), env_global, L!("33").to_owned());
ts.handle_columns_lines_var_change(&*parser.get_vars());
assert_eq!(ts.last(), Termsize::new(33, 150));
// Oh it got SIGWINCH, now the tty matters again.
TermsizeContainer::handle_winch();
assert_eq!(ts.last(), Termsize::new(33, 150));
assert_eq!(ts.updating(parser), stubby_termsize().unwrap());
assert_eq!(parser.var_as_string(L!("COLUMNS")).unwrap(), "42");
assert_eq!(parser.var_as_string(L!("LINES")).unwrap(), "84");
assert_eq!(vars.get(L!("COLUMNS")).unwrap().as_string(), "42");
assert_eq!(vars.get(L!("LINES")).unwrap().as_string(), "84");
// Test initialize().
parser.set_var(L!("COLUMNS"), &[L!("83")], env_global);
parser.set_var(L!("LINES"), &[L!("38")], env_global);
ts.initialize(parser.get_var_stack_env());
vars.set_one(L!("COLUMNS"), env_global, L!("83").to_owned());
vars.set_one(L!("LINES"), env_global, L!("38").to_owned());
ts.initialize(&*vars);
assert_eq!(ts.last(), Termsize::new(83, 38));
// initialize() even beats the tty reader until a sigwinch.
@ -409,7 +404,7 @@ add_test!("test_termsize", || {
setting_env_vars: AtomicBool::new(false),
tty_size_reader: stubby_termsize,
};
ts.initialize(parser.get_var_stack_env());
ts.initialize(&*parser.get_vars());
ts2.updating(parser);
assert_eq!(ts.last(), Termsize::new(83, 38));
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.
environment_t &env_vars = vars;
auto termsize = termsize_initialize_ffi(reinterpret_cast<const unsigned char *>(&env_vars));
auto termsize =
termsize_initialize_ffi(reinterpret_cast<const unsigned char *>(vars.get_impl_ffi()));
if (!vars.get_unless_empty(L"COLUMNS"))
vars.set_one(L"COLUMNS", ENV_GLOBAL, to_string(termsize.width));
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.
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:
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; }
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); }
/// Get the library data.