Follow API guidelines for public types (#12283)

# Description
Follow the [API guideline naming
conventions](https://rust-lang.github.io/api-guidelines/naming.html)
also for our externally exposed types

(See
[`clippy::wrong_self_convention`](https://rust-lang.github.io/rust-clippy/master/index.html#/wrong_self_convention)
with [`avoid-breaking-exported-api =
false`](https://doc.rust-lang.org/clippy/lint_configuration.html#avoid-breaking-exported-api)
)

Also be a good citizen around doccomments

- **Fix `Unit::to_value` to `Unit::build_value`**
- **Fix `PipelineData::is_external_failed` to `check_external_failed`**
- **Fix doccomment on `check_external_failed`**
- **Fix `Value::into_config` naming to `parse_as_config`**
- **Document `Value::parse_as_config`**

# Plugin-Author-Facing Changes
See renames above
This commit is contained in:
Stefan Holderbach 2024-03-26 12:12:25 +01:00 committed by GitHub
parent e8bcfbaed1
commit 24d2c8dd8e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 24 additions and 15 deletions

View File

@ -65,7 +65,7 @@ impl Command for Try {
}
// external command may fail to run
Ok(pipeline) => {
let (pipeline, external_failed) = pipeline.is_external_failed();
let (pipeline, external_failed) = pipeline.check_external_failed();
if external_failed {
// Because external command errors aren't "real" errors,
// (unless do -c is in effect)

View File

@ -353,7 +353,7 @@ pub fn find_in_dirs_env(
/// is the canonical way to fetch config at runtime when you have Stack available.
pub fn get_config(engine_state: &EngineState, stack: &Stack) -> Config {
if let Some(mut config_record) = stack.get_env_var(engine_state, "config") {
config_record.into_config(engine_state.get_config()).0
config_record.parse_as_config(engine_state.get_config()).0
} else {
engine_state.get_config().clone()
}

View File

@ -314,7 +314,7 @@ pub fn eval_expression_with_input<D: DebugContext>(
// Try to catch and detect if external command runs to failed.
fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) {
input.is_external_failed()
input.check_external_failed()
}
fn eval_redirection<D: DebugContext>(

View File

@ -32,7 +32,7 @@ pub enum Unit {
}
impl Unit {
pub fn to_value(&self, size: i64, span: Span) -> Result<Value, ShellError> {
pub fn build_value(self, size: i64, span: Span) -> Result<Value, ShellError> {
match self {
Unit::Byte => Ok(Value::filesize(size, span)),
Unit::Kilobyte => Ok(Value::filesize(size * 1000, span)),

View File

@ -172,9 +172,16 @@ impl Default for Config {
}
impl Value {
pub fn into_config(&mut self, config: &Config) -> (Config, Option<ShellError>) {
/// Parse the given [`Value`] as a configuration record, and recover encountered mistakes
///
/// If any given (sub)value is detected as impossible, this value will be restored to the value
/// in `existing_config`, thus mutates `self`.
///
/// Returns a new [`Config`] (that is in a valid state) and if encountered the [`ShellError`]
/// containing all observed inner errors.
pub fn parse_as_config(&mut self, existing_config: &Config) -> (Config, Option<ShellError>) {
// Clone the passed-in config rather than mutating it.
let mut config = config.clone();
let mut config = existing_config.clone();
// Vec for storing errors. Current Nushell behaviour (Dec 2022) is that having some typo
// like `"always_trash": tru` in your config.nu's `$env.config` record shouldn't abort all

View File

@ -295,7 +295,7 @@ impl EngineState {
// Don't insert the record as the "config" env var as-is.
// Instead, mutate a clone of it with into_config(), and put THAT in env_vars.
let mut new_record = v.clone();
let (config, error) = new_record.into_config(&self.config);
let (config, error) = new_record.parse_as_config(&self.config);
self.config = Arc::new(config);
config_updated = true;
env_vars.insert(k, new_record);

View File

@ -145,7 +145,7 @@ impl Matcher for Pattern {
let span = unit.span;
if let Expr::Int(size) = amount.expr {
match &unit.item.to_value(size, span) {
match &unit.item.build_value(size, span) {
Ok(v) => v == value,
_ => false,
}

View File

@ -136,7 +136,7 @@ pub trait Eval {
Expr::String(s) => Ok(Value::string(s.clone(), expr.span)),
Expr::Nothing => Ok(Value::nothing(expr.span)),
Expr::ValueWithUnit(e, unit) => match Self::eval::<D>(state, mut_state, e)? {
Value::Int { val, .. } => unit.item.to_value(val, unit.span),
Value::Int { val, .. } => unit.item.build_value(val, unit.span),
x => Err(ShellError::CantConvert {
to_type: "unit value".into(),
from_type: x.get_type().to_string(),

View File

@ -699,14 +699,16 @@ impl PipelineData {
}
}
/// Try to catch external stream exit status and detect if it runs to failed.
/// Try to catch the external stream exit status and detect if it failed.
///
/// This is useful to commands with semicolon, we can detect errors early to avoid
/// commands after semicolon running.
/// This is useful for external commands with semicolon, we can detect errors early to avoid
/// commands after the semicolon running.
///
/// Returns self and a flag indicates if the external stream runs to failed.
/// If `self` is not Pipeline::ExternalStream, the flag will be false.
pub fn is_external_failed(self) -> (Self, bool) {
/// Returns `self` and a flag that indicates if the external stream run failed. If `self` is
/// not [`PipelineData::ExternalStream`], the flag will be `false`.
///
/// Currently this will consume an external stream to completion.
pub fn check_external_failed(self) -> (Self, bool) {
let mut failed_to_run = false;
// Only need ExternalStream without redirecting output.
// It indicates we have no more commands to execute currently.