Skip to content

Commit

Permalink
Use Record::get instead of Value functions (#10925)
Browse files Browse the repository at this point in the history 
# Description
Where appropriate, this PR replaces instances of
`Value::get_data_by_key` and `Value::follow_cell_path` with
`Record::get`. This avoids some unnecessary clones and simplifies the
code in some places.
  • Loading branch information
@IanManske
IanManske committed Nov 8, 2023
1 parent 435abad commit 59ea28c
Show file tree
Hide file tree
Showing 17 changed files with 231 additions and 290 deletions.
21 changes: 9 additions & 12 deletions crates/nu-cli/src/completions/custom_completions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,20 @@ impl Completer for CustomCompletion {
.map(|pd| {
let value = pd.into_value(span);
match &value {
Value::Record { .. } => {
let completions = value
.get_data_by_key("completions")
Value::Record { val, .. } => {
let completions = val
.get("completions")
.and_then(|val| {
val.as_list()
.ok()
.map(|it| map_value_completions(it.iter(), span, offset))
})
.unwrap_or_default();
let options = value.get_data_by_key("options");
let options = val.get("options");

if let Some(Value::Record { .. }) = &options {
let options = options.unwrap_or_default();
if let Some(Value::Record { val: options, .. }) = &options {
let should_sort = options
.get_data_by_key("sort")
.get("sort")
.and_then(|val| val.as_bool().ok())
.unwrap_or(false);

Expand All @@ -103,21 +102,19 @@ impl Completer for CustomCompletion {

custom_completion_options = Some(CompletionOptions {
case_sensitive: options
.get_data_by_key("case_sensitive")
.get("case_sensitive")
.and_then(|val| val.as_bool().ok())
.unwrap_or(true),
positional: options
.get_data_by_key("positional")
.get("positional")
.and_then(|val| val.as_bool().ok())
.unwrap_or(true),
sort_by: if should_sort {
SortBy::Ascending
} else {
SortBy::None
},
match_algorithm: match options
.get_data_by_key("completion_algorithm")
{
match_algorithm: match options.get("completion_algorithm") {
Some(option) => option
.as_string()
.ok()
Expand Down
28 changes: 11 additions & 17 deletions crates/nu-cli/src/menus/menu_completions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,24 +80,18 @@ fn convert_to_suggestions(
only_buffer_difference: bool,
) -> Vec<Suggestion> {
match value {
Value::Record { .. } => {
let text = value
.get_data_by_key("value")
Value::Record { val, .. } => {
let text = val
.get("value")
.and_then(|val| val.as_string().ok())
.unwrap_or_else(|| "No value key".to_string());

let description = value
.get_data_by_key("description")
.and_then(|val| val.as_string().ok());
let description = val.get("description").and_then(|val| val.as_string().ok());

let span = match value.get_data_by_key("span") {
Some(span @ Value::Record { .. }) => {
let start = span
.get_data_by_key("start")
.and_then(|val| val.as_int().ok());
let end = span
.get_data_by_key("end")
.and_then(|val| val.as_int().ok());
let span = match val.get("span") {
Some(Value::Record { val: span, .. }) => {
let start = span.get("start").and_then(|val| val.as_int().ok());
let end = span.get("end").and_then(|val| val.as_int().ok());
match (start, end) {
(Some(start), Some(end)) => {
let start = start.min(end);
Expand Down Expand Up @@ -126,12 +120,12 @@ fn convert_to_suggestions(
},
};

let extra = match value.get_data_by_key("extra") {
let extra = match val.get("extra") {
Some(Value::List { vals, .. }) => {
let extra: Vec<String> = vals
.into_iter()
.iter()
.filter_map(|extra| match extra {
Value::String { val, .. } => Some(val),
Value::String { val, .. } => Some(val.clone()),
_ => None,
})
.collect();
Expand Down
44 changes: 17 additions & 27 deletions crates/nu-cmd-base/src/hook.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::util::get_guaranteed_cwd;
use miette::Result;
use nu_engine::{eval_block, eval_block_with_early_return};
use nu_parser::parse;
use nu_protocol::ast::PathMember;
use nu_protocol::cli_error::{report_error, report_error_new};
use nu_protocol::engine::{EngineState, Stack, StateWorkingSet};
use nu_protocol::{BlockId, PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId};
Expand Down Expand Up @@ -62,28 +61,8 @@ pub fn eval_hook(
value: &Value,
hook_name: &str,
) -> Result<PipelineData, ShellError> {
let value_span = value.span();

// Hooks can optionally be a record in this form:
// {
// condition: {|before, after| ... } # block that evaluates to true/false
// code: # block or a string
// }
// The condition block will be run to check whether the main hook (in `code`) should be run.
// If it returns true (the default if a condition block is not specified), the hook should be run.
let condition_path = PathMember::String {
val: "condition".to_string(),
span: value_span,
optional: false,
};
let mut output = PipelineData::empty();

let code_path = PathMember::String {
val: "code".to_string(),
span: value_span,
optional: false,
};

let span = value.span();
match value {
Value::String { val, .. } => {
Expand Down Expand Up @@ -161,10 +140,15 @@ pub fn eval_hook(
)?;
}
}
Value::Record { .. } => {
let do_run_hook = if let Ok(condition) =
value.clone().follow_cell_path(&[condition_path], false)
{
Value::Record { val, .. } => {
// Hooks can optionally be a record in this form:
// {
// condition: {|before, after| ... } # block that evaluates to true/false
// code: # block or a string
// }
// The condition block will be run to check whether the main hook (in `code`) should be run.
// If it returns true (the default if a condition block is not specified), the hook should be run.
let do_run_hook = if let Some(condition) = val.get("condition") {
let other_span = condition.span();
if let Ok(block_id) = condition.as_block() {
match run_hook_block(
Expand Down Expand Up @@ -204,7 +188,13 @@ pub fn eval_hook(
};

if do_run_hook {
let follow = value.clone().follow_cell_path(&[code_path], false)?;
let Some(follow) = val.get("code") else {
return Err(ShellError::CantFindColumn {
col_name: "code".into(),
span,
src_span: span,
});
};
let source_span = follow.span();
match follow {
Value::String { val, .. } => {
Expand Down Expand Up @@ -270,7 +260,7 @@ pub fn eval_hook(
run_hook_block(
engine_state,
stack,
block_id,
*block_id,
input,
arguments,
source_span,
Expand Down
12 changes: 6 additions & 6 deletions crates/nu-cmd-extra/src/extra/formats/to/html.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,15 +402,15 @@ fn html_table(table: Vec<Value>, headers: Vec<String>, config: &Config) -> Strin

for row in table {
let span = row.span();
if let Value::Record { .. } = row {
if let Value::Record { val: row, .. } = row {
output_string.push_str("<tr>");
for header in &headers {
let data = row.get_data_by_key(header);
let data = row
.get(header)
.cloned()
.unwrap_or_else(|| Value::nothing(span));
output_string.push_str("<td>");
output_string.push_str(&html_value(
data.unwrap_or_else(|| Value::nothing(span)),
config,
));
output_string.push_str(&html_value(data, config));
output_string.push_str("</td>");
}
output_string.push_str("</tr>");
Expand Down
62 changes: 36 additions & 26 deletions crates/nu-cmd-lang/src/core_commands/error_make.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use nu_engine::CallExt;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value,
Category, Example, PipelineData, Record, ShellError, Signature, Span, SyntaxShape, Type, Value,
};

#[derive(Clone)]
Expand Down Expand Up @@ -118,8 +118,9 @@ impl Command for ErrorMake {
const UNABLE_TO_PARSE: &str = "Unable to parse error format.";

fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
let span = value.span();
let value = match value {
Value::Record { .. } => value,
Value::Record { val, .. } => val,
_ => {
return ShellError::GenericError(
"Creating error value not supported.".into(),
Expand All @@ -131,13 +132,13 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
}
};

let msg = match value.get_data_by_key("msg") {
Some(Value::String { val, .. }) => val,
let msg = match value.get("msg") {
Some(Value::String { val, .. }) => val.clone(),
Some(_) => {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"`$.msg` has wrong type, must be string".into(),
Some(value.span()),
Some(span),
None,
Vec::new(),
)
Expand All @@ -146,20 +147,29 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"missing required member `$.msg`".into(),
Some(value.span()),
Some(span),
None,
Vec::new(),
)
}
};

let help = match value.get_data_by_key("help") {
Some(Value::String { val, .. }) => Some(val),
let help = match value.get("help") {
Some(Value::String { val, .. }) => Some(val.clone()),
_ => None,
};

let label = match value.get_data_by_key("label") {
Some(value) => value,
let (label, label_span) = match value.get("label") {
Some(value @ Value::Record { val, .. }) => (val, value.span()),
Some(_) => {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"`$.label` has wrong type, must be a record".into(),
Some(span),
None,
Vec::new(),
)
}
// correct return: no label
None => {
return ShellError::GenericError(
Expand All @@ -173,23 +183,23 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
};

// remove after a few versions
if label.get_data_by_key("start").is_some() || label.get_data_by_key("end").is_some() {
if label.get("start").is_some() || label.get("end").is_some() {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"`start` and `end` are deprecated".into(),
Some(value.span()),
Some(span),
Some("Use `$.label.span` instead".into()),
Vec::new(),
);
}

let text = match label.get_data_by_key("text") {
Some(Value::String { val, .. }) => val,
let text = match label.get("text") {
Some(Value::String { val, .. }) => val.clone(),
Some(_) => {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"`$.label.text` has wrong type, must be string".into(),
Some(label.span()),
Some(label_span),
None,
Vec::new(),
)
Expand All @@ -198,15 +208,15 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
"missing required member `$.label.text`".into(),
Some(label.span()),
Some(label_span),
None,
Vec::new(),
)
}
};

let span = match label.get_data_by_key("span") {
Some(val @ Value::Record { .. }) => val,
let (span, span_span) = match label.get("span") {
Some(value @ Value::Record { val, .. }) => (val, value.span()),
Some(value) => {
return ShellError::GenericError(
UNABLE_TO_PARSE.into(),
Expand All @@ -220,11 +230,11 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
None => return ShellError::GenericError(msg, text, throw_span, help, Vec::new()),
};

let span_start = match get_span_sides(&span, "start") {
let span_start = match get_span_sides(span, span_span, "start") {
Ok(val) => val,
Err(err) => return err,
};
let span_end = match get_span_sides(&span, "end") {
let span_end = match get_span_sides(span, span_span, "end") {
Ok(val) => val,
Err(err) => return err,
};
Expand All @@ -233,7 +243,7 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
return ShellError::GenericError(
"invalid error format.".into(),
"`$.label.start` should be smaller than `$.label.end`".into(),
Some(label.span()),
Some(label_span),
Some(format!("{} > {}", span_start, span_end)),
Vec::new(),
);
Expand All @@ -249,20 +259,20 @@ fn make_other_error(value: &Value, throw_span: Option<Span>) -> ShellError {
)
}

fn get_span_sides(span: &Value, side: &str) -> Result<i64, ShellError> {
match span.get_data_by_key(side) {
Some(Value::Int { val, .. }) => Ok(val),
fn get_span_sides(span: &Record, span_span: Span, side: &str) -> Result<i64, ShellError> {
match span.get(side) {
Some(Value::Int { val, .. }) => Ok(*val),
Some(_) => Err(ShellError::GenericError(
UNABLE_TO_PARSE.into(),
format!("`$.span.{side}` must be int"),
Some(span.span()),
Some(span_span),
None,
Vec::new(),
)),
None => Err(ShellError::GenericError(
UNABLE_TO_PARSE.into(),
format!("`$.span.{side}` must be present, if span is specified."),
Some(span.span()),
Some(span_span),
None,
Vec::new(),
)),
Expand Down

0 comments on commit 59ea28c

Please sign in to comment.