Skip to content

Commit

Permalink
Add VariableSet::get_scalar
Browse files Browse the repository at this point in the history
  • Loading branch information
magicant committed Jul 9, 2024
1 parent bf82bc9 commit dc0779e
Show file tree
Hide file tree
Showing 13 changed files with 72 additions and 92 deletions.
9 changes: 1 addition & 8 deletions yash-builtin/src/cd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ use crate::common::report_failure;
use crate::Result;
use std::path::Path;
use yash_env::semantics::Field;
use yash_env::variable::Value::Scalar;
use yash_env::variable::PWD;
use yash_env::Env;

Expand Down Expand Up @@ -186,13 +185,7 @@ pub mod syntax;
pub mod target;

fn get_pwd(env: &Env) -> String {
match env.variables.get(PWD) {
Some(variable) => match &variable.value {
Some(Scalar(value)) => value.clone(),
_ => String::new(),
},
None => String::new(),
}
env.variables.get_scalar(PWD).unwrap_or_default().to_owned()
}

/// Entry point for executing the `cd` built-in
Expand Down
9 changes: 3 additions & 6 deletions yash-builtin/src/cd/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use std::borrow::Cow;
use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;
use yash_env::variable::Value;
use yash_env::variable::HOME;
use yash_env::variable::OLDPWD;
use yash_env::Env;
Expand Down Expand Up @@ -135,11 +134,9 @@ impl MessageBase for TargetError {

/// Returns the variable value if it is a non-empty scalar.
fn get_scalar<'a>(env: &'a Env, name: &str) -> Option<&'a str> {
let var = env.variables.get(name)?;
match &var.value {
Some(Value::Scalar(value)) if !value.is_empty() => Some(value),
_ => None,
}
env.variables
.get_scalar(name)
.filter(|value| !value.is_empty())
}

/// Computes the target directory of the cd built-in.
Expand Down
10 changes: 1 addition & 9 deletions yash-builtin/src/getopts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ use std::num::NonZeroUsize;
use yash_env::builtin::getopts::Origin;
use yash_env::semantics::ExitStatus;
use yash_env::semantics::Field;
use yash_env::variable::Value;
use yash_env::variable::OPTIND;
use yash_env::Env;

Expand Down Expand Up @@ -248,14 +247,7 @@ pub async fn main(env: &mut Env, args: Vec<Field>) -> crate::Result {
};

// Get the `$OPTIND` value
let optind = env
.variables
.get(OPTIND)
.and_then(|v| match &v.value {
Some(Value::Scalar(value)) => Some(value.as_str()),
_ => None,
})
.unwrap_or("");
let optind = env.variables.get_scalar(OPTIND).unwrap_or_default();
let (arg_index, char_index) = indexes_from_optind(optind);

// Verify the state
Expand Down
16 changes: 7 additions & 9 deletions yash-builtin/src/read/assigning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

use yash_env::semantics::Field;
use yash_env::variable::Scope;
use yash_env::variable::Value;
use yash_env::variable::Variable;
use yash_env::variable::IFS;
use yash_env::Env;
use yash_semantics::expansion::attr::AttrChar;
Expand All @@ -46,13 +44,11 @@ pub fn assign(
variables: Vec<Field>,
last_variable: Field,
) -> Vec<Error> {
#[rustfmt::skip]
let ifs = match env.variables.get(IFS) {
Some(&Variable { value: Some(Value::Scalar(ref value)), .. }) => value,
// TODO If the variable is an array, should we ignore it?
_ => Ifs::DEFAULT,
};
let ifs = ifs.to_owned();
let ifs = env
.variables
.get_scalar(IFS)
.unwrap_or(Ifs::DEFAULT)
.to_owned();
let ifs = Ifs::new(&ifs);

let mut ranges = ifs.ranges(text.iter().copied());
Expand Down Expand Up @@ -107,6 +103,8 @@ fn assign_one(env: &mut Env, name: Field, value: &[AttrChar]) -> Result<(), Erro
mod tests {
use super::*;
use assert_matches::assert_matches;
use yash_env::variable::Value;
use yash_env::variable::Variable;
use yash_env::variable::VariableSet;
use yash_semantics::expansion::attr::Origin;
use yash_syntax::source::Location;
Expand Down
2 changes: 2 additions & 0 deletions yash-env/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `input::Echo`
- This is a decorator of `Input` that implements the behavior of the verbose shell option.
- `input::FdReader` is now marked `#[must_use]`.
- `variable::VariableSet::get_scalar`
- This is a convenience method that returns a scalar variable as a `Cow<str>`.
- Variable name constants in the `variable` module:
`CDPATH`, `ENV`, `HOME`, `IFS`, `LINENO`, `OLDPWD`, `OPTARG`, `OPTIND`,
`PATH`, `PPID`, `PS1`, `PS2`, `PS4`, `PWD`
Expand Down
32 changes: 18 additions & 14 deletions yash-env/src/pwd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ use crate::system::Errno;
use crate::system::AT_FDCWD;
use crate::variable::AssignError;
use crate::variable::Scope::Global;
use crate::variable::Value::Scalar;
use crate::variable::Variable;
use crate::variable::PWD;
use crate::System;
use nix::sys::stat::FileStat;
Expand Down Expand Up @@ -64,19 +62,25 @@ impl Env {
/// - there is no dot (`.`) or dot-dot (`..`) component in the pathname.
#[must_use]
pub fn get_pwd_if_correct(&self) -> Option<&str> {
match self.variables.get(PWD) {
Some(Variable {
value: Some(Scalar(pwd)),
..
}) if Path::new(pwd).is_absolute() && !has_dot_or_dot_dot(pwd) => {
const AT_FLAGS: AtFlags = AtFlags::empty();
let cstr_pwd = CString::new(pwd.as_bytes()).ok()?;
let s1 = self.system.fstatat(AT_FDCWD, &cstr_pwd, AT_FLAGS).ok()?;
let s2 = self.system.fstatat(AT_FDCWD, c".", AT_FLAGS).ok()?;
same_files(&s1, &s2).then_some(pwd)
self.variables.get_scalar(PWD).filter(|pwd| {
if !Path::new(pwd).is_absolute() {
return false;
}
_ => None,
}
if has_dot_or_dot_dot(pwd) {
return false;
}
let Ok(cstr_pwd) = CString::new(pwd.as_bytes()) else {
return false;
};
const AT_FLAGS: AtFlags = AtFlags::empty();
let Ok(s1) = self.system.fstatat(AT_FDCWD, &cstr_pwd, AT_FLAGS) else {
return false;
};
let Ok(s2) = self.system.fstatat(AT_FDCWD, c".", AT_FLAGS) else {
return false;
};
same_files(&s1, &s2)
})
}

/// Tests if the `$PWD` variable is correct.
Expand Down
23 changes: 23 additions & 0 deletions yash-env/src/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,29 @@ impl VariableSet {
}
}

/// Gets the value of the specified scalar variable.
///
/// This is a convenience function that retrieves the value of the specified
/// scalar variable. If the variable is unset or an array, this method
/// returns `None`.
///
/// Note that this function does not apply any [`Quirk`] the variable may
/// have. Use [`Variable::expand`] to apply quirks.
#[must_use]
pub fn get_scalar<N>(&self, name: &N) -> Option<&str>
where
String: Borrow<N>,
N: Hash + Eq + ?Sized,
{
fn inner(var: &Variable) -> Option<&str> {
match var.value.as_ref()? {
Scalar(value) => Some(value),
Array(_) => None,
}
}
inner(self.get(name)?)
}

/// Unsets a variable.
///
/// If successful, the return value is the previous value. If the specified
Expand Down
11 changes: 2 additions & 9 deletions yash-prompt/src/prompter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
use async_trait::async_trait;
use std::cell::RefCell;
use yash_env::input::{Context, Input, Result};
use yash_env::variable::{Expansion, VariableSet, PS1, PS2};
use yash_env::variable::{VariableSet, PS1, PS2};
use yash_env::Env;
use yash_syntax::source::Location;

/// [`Input`] decorator that shows a command prompt
///
Expand Down Expand Up @@ -82,13 +81,7 @@ async fn print_prompt(env: &mut Env, context: &Context) {
/// This function does not consider yash-specific prompt variables.
pub fn fetch_posix(variables: &VariableSet, context: &Context) -> String {
let var = if context.is_first_line() { PS1 } else { PS2 };
// The location is irrelevant (as long as the variable does not have a quirk)
let location = Location::dummy(String::new());
// https://github.com/rust-lang/rust-clippy/issues/13031
match variables.get(var).map(|v| v.expand(&location)) {
Some(Expansion::Scalar(s)) => s.into_owned(),
_ => Default::default(),
}
variables.get_scalar(var).unwrap_or_default().to_owned()
}

// TODO pub fn fetch_ex: yash-specific prompt variables
Expand Down
14 changes: 6 additions & 8 deletions yash-semantics/src/expansion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ use thiserror::Error;
use yash_env::semantics::ExitStatus;
use yash_env::system::Errno;
use yash_env::variable::Value;
use yash_env::variable::Variable;
use yash_env::variable::IFS;
use yash_syntax::source::pretty::Annotation;
use yash_syntax::source::pretty::AnnotationType;
Expand Down Expand Up @@ -316,13 +315,12 @@ pub async fn expand_words<'a, I: IntoIterator<Item = &'a Word>>(
// TODO brace expansion

// field splitting //
use yash_env::variable::Value::Scalar;
#[rustfmt::skip]
let ifs = match env.inner.variables.get(IFS) {
Some(&Variable { value: Some(Scalar(ref value)), .. }) => Ifs::new(value),
// TODO If the variable is an array, should we ignore it?
_ => Ifs::default(),
};
let ifs = env
.inner
.variables
.get_scalar(IFS)
.map(Ifs::new)
.unwrap_or_default();
let mut split_fields = Vec::with_capacity(fields.len());
for field in fields {
split::split_into(field, &ifs, &mut split_fields);
Expand Down
13 changes: 5 additions & 8 deletions yash-semantics/src/expansion/initial/arith.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ use yash_arith::eval;
use yash_env::option::Option::Unset;
use yash_env::option::State::{Off, On};
use yash_env::variable::Scope::Global;
use yash_env::variable::Value::Scalar;
use yash_env::variable::Variable;
use yash_syntax::source::Code;
use yash_syntax::source::Location;
use yash_syntax::source::Source;
Expand Down Expand Up @@ -208,17 +206,15 @@ impl<'a> yash_arith::Env for VarEnv<'a> {
type GetVariableError = UnsetVariable;
type AssignVariableError = AssignReadOnlyError;

#[rustfmt::skip]
fn get_variable(&self, name: &str) -> Result<Option<&str>, UnsetVariable> {
if let Some(Variable { value: Some(Scalar(value)), .. }) = self.env.variables.get(name) {
Ok(Some(value))
} else {
match self.env.options.get(Unset) {
match self.env.variables.get_scalar(name) {
Some(value) => Ok(Some(value)),
None => match self.env.options.get(Unset) {
// TODO If the variable exists but is not scalar, UnsetVariable
// does not seem to be the right error.
Off => Err(UnsetVariable),
On => Ok(None),
}
},
}
}

Expand Down Expand Up @@ -305,6 +301,7 @@ mod tests {
use yash_env::semantics::ExitStatus;
use yash_env::system::Errno;
use yash_env::variable::Scope::Global;
use yash_env::variable::Value::Scalar;
use yash_env_test_helper::in_virtual_system;

#[test]
Expand Down
3 changes: 1 addition & 2 deletions yash-semantics/src/expansion/initial/param/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ pub fn resolve<'a>(name: Name<'_>, env: &'a Env, location: &Location) -> Expansi
fn variable<'a>(env: &'a Env, name: &str, location: &Location) -> Expansion<'a> {
env.variables
.get(name)
.map(|v| v.expand(location))
.unwrap_or(Expansion::Unset)
.map_or(Expansion::Unset, |v| v.expand(location))
}
fn options(env: &Env) -> Expansion {
let mut value = String::new();
Expand Down
11 changes: 2 additions & 9 deletions yash-semantics/src/expansion/initial/tilde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

use crate::expansion::attr::AttrChar;
use crate::expansion::attr::Origin;
use yash_env::variable::Value;
use yash_env::variable::Variable;
use yash_env::variable::HOME;
use yash_env::Env;
use yash_env::System;
Expand All @@ -41,13 +39,7 @@ where
/// Performs tilde expansion.
pub fn expand(name: &str, env: &Env) -> Vec<AttrChar> {
if name.is_empty() {
let result = match env.variables.get(HOME) {
Some(Variable {
value: Some(Value::Scalar(value)),
..
}) => value,
_ => "~",
};
let result = env.variables.get_scalar(HOME).unwrap_or("~");
into_attr_chars(result.chars())
} else {
if let Ok(Some(path)) = env.system.getpwnam_dir(name) {
Expand All @@ -64,6 +56,7 @@ mod tests {
use super::*;
use std::path::PathBuf;
use yash_env::variable::Scope;
use yash_env::variable::Value;
use yash_env::VirtualSystem;

#[test]
Expand Down
11 changes: 1 addition & 10 deletions yash-semantics/src/xtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,13 @@ use std::fmt::Write;
use yash_env::option::OptionSet;
use yash_env::option::State;
use yash_env::semantics::Field;
use yash_env::variable::Value::Scalar;
use yash_env::variable::Variable;
use yash_env::variable::PS4;
use yash_env::Env;
use yash_quote::quoted;
use yash_syntax::syntax::Text;

async fn expand_ps4(env: &mut Env) -> String {
let value = match env.variables.get(PS4) {
Some(Variable {
value: Some(Scalar(value)),
..
}) => value.to_owned(),

_ => String::new(),
};
let value = env.variables.get_scalar(PS4).unwrap_or_default().to_owned();

let text = match value.parse::<Text>() {
Ok(text) => text,
Expand Down

0 comments on commit dc0779e

Please sign in to comment.