Skip to content

Commit

Permalink
Use record API in more parts of nu-protocol (nushell#10928)
Browse files Browse the repository at this point in the history
# Description

This is pretty complementary/orthogonal to @IanManske 's changes to
`Value` cellpath accessors in:
- nushell#10925
- to a lesser extent nushell#10926

## Steps
- Use `R.remove` in `Value.remove_data_at_cell_path`
- Pretty sound after nushell#10875 (tests mentioned in commit message have been
removed by that)
- Update `did_you_mean` helper to use iterator
- Change `Value::columns` to return iterator
  - This is not a place of honor
- Use `Record::get` in `Value::get_data_by_key`
# User-Facing Changes
None intentional, potential edge cases on duplicated columns could
change (considered undefined behavior)

# Tests + Formatting
(-)
  • Loading branch information
sholderbach authored and hardfau1t committed Dec 14, 2023
1 parent 1f600b6 commit bb76881
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 38 deletions.
2 changes: 1 addition & 1 deletion crates/nu-cmd-extra/src/extra/filters/update_cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl Iterator for UpdateCellIterator {
match self.input.next() {
Some(val) => {
if let Some(ref cols) = self.columns {
if !val.columns().iter().any(|c| cols.contains(c)) {
if !val.columns().any(|c| cols.contains(c)) {
return Some(val);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/conversions/into/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Iterator for UpdateCellIterator {
match self.input.next() {
Some(val) => {
if let Some(ref cols) = self.columns {
if !val.columns().iter().any(|c| cols.contains(c)) {
if !val.columns().any(|c| cols.contains(c)) {
return Some(val);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/formats/to/nuon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ pub fn value_to_string(
Value::Int { val, .. } => Ok(format!("{}", *val)),
Value::List { vals, .. } => {
let headers = get_columns(vals);
if !headers.is_empty() && vals.iter().all(|x| x.columns() == headers) {
if !headers.is_empty() && vals.iter().all(|x| x.columns().eq(headers.iter())) {
// Table output
let headers: Vec<String> = headers
.iter()
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-command/src/viewers/griddle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ fn convert_to_list(
let mut iter = iter.into_iter().peekable();

if let Some(first) = iter.peek() {
let mut headers = first.columns().to_vec();
let mut headers: Vec<String> = first.columns().cloned().collect();

if !headers.is_empty() {
headers.insert(0, "#".into());
Expand Down
8 changes: 6 additions & 2 deletions crates/nu-protocol/src/did_you_mean.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
pub fn did_you_mean<S: AsRef<str>>(possibilities: &[S], input: &str) -> Option<String> {
let possibilities: Vec<&str> = possibilities.iter().map(|s| s.as_ref()).collect();
pub fn did_you_mean<'a, 'b, I, S>(possibilities: I, input: &'b str) -> Option<String>
where
I: IntoIterator<Item = &'a S>,
S: AsRef<str> + 'a + ?Sized,
{
let possibilities: Vec<&str> = possibilities.into_iter().map(|s| s.as_ref()).collect();
let suggestion =
crate::lev_distance::find_best_match_for_name_with_substrings(&possibilities, input, None)
.map(|s| s.to_string());
Expand Down
44 changes: 12 additions & 32 deletions crates/nu-protocol/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,9 @@ impl Value {
return Ok(Value::nothing(*origin_span)); // short-circuit
} else {
if from_user_input {
if let Some(suggestion) = did_you_mean(&val.cols, column_name) {
if let Some(suggestion) =
did_you_mean(val.columns(), column_name)
{
return Err(ShellError::DidYouMean(
suggestion,
*origin_span,
Expand Down Expand Up @@ -1420,19 +1422,7 @@ impl Value {

match val {
Value::Record { val: record, .. } => {
let mut found = false;
let mut index = 0;
record.cols.retain_mut(|col| {
if col == col_name {
found = true;
record.vals.remove(index);
false
} else {
index += 1;
true
}
});
if !found && !optional {
if record.remove(col_name).is_none() && !optional {
return Err(ShellError::CantFindColumn {
col_name: col_name.to_string(),
span: *span,
Expand All @@ -1452,19 +1442,7 @@ impl Value {
Ok(())
}
Value::Record { val: record, .. } => {
let mut found = false;
let mut index = 0;
record.cols.retain_mut(|col| {
if col == col_name {
found = true;
record.vals.remove(index);
false
} else {
index += 1;
true
}
});
if !found && !optional {
if record.remove(col_name).is_none() && !optional {
return Err(ShellError::CantFindColumn {
col_name: col_name.to_string(),
span: *span,
Expand Down Expand Up @@ -1741,11 +1719,13 @@ impl Value {
matches!(self, Value::Bool { val: false, .. })
}

pub fn columns(&self) -> &[String] {
match self {
Value::Record { val, .. } => &val.cols,
_ => &[],
}
pub fn columns(&self) -> impl Iterator<Item = &String> {
let opt = match self {
Value::Record { val, .. } => Some(val.columns()),
_ => None,
};

opt.into_iter().flatten()
}

pub fn bool(val: bool, span: Span) -> Value {
Expand Down

0 comments on commit bb76881

Please sign in to comment.