holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

remove --column from length command and remove record processing (#10091)

Browse Source 
# Description

This PR removes `record` processing from the `length` command. It just
doesn't make sense to try and get the length of a record. This PR also
removes the `--column` parameter. If you want to list or count columns,
you could use `$table | columns` or `$table | columns | length`.

close #10074 

### Before

![image](https://github.com/nushell/nushell/assets/343840/83488316-3ec4-4c32-9583-00341a71f46f)

### After
Catches records two different ways now.
with the `input_output_types` checker

![image](https://github.com/nushell/nushell/assets/343840/ca67f8b6-359e-4933-ab4d-1b702f8d79cf)

and with additional logic in the command for cases like `echo`

![image](https://github.com/nushell/nushell/assets/343840/99064351-b208-4bd3-bab9-535f97cd7ad4)


# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
Darren Schroeder 2023-08-23 16:03:26 -05:00 committed by GitHub
parent 3d698b74d8
commit af82eeca72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 32 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
crates/nu-command/src/filters/length.rs
View File

@ -1,9 +1,7 @@
use nu_engine::column::get_columns;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, ShellError,
Signature, Span, Type, Value,
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Type, Value,
};
#[derive(Clone)]
@ -15,7 +13,7 @@ impl Command for Length {
}
fn usage(&self) -> &str {
"Count the number of elements in the input."
"Count the number of items in an input list or rows in a table."
}
fn signature(&self) -> nu_protocol::Signature {
@ -24,7 +22,6 @@ impl Command for Length {
(Type::List(Box::new(Type::Any)), Type::Int),
(Type::Table(vec![]), Type::Int),
])
.switch("column", "Show the number of columns in a table", Some('c'))
.category(Category::Filters)
}
@ -34,17 +31,12 @@ impl Command for Length {
fn run(
&self,
engine_state: &EngineState,
_engine_state: &EngineState,
_stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let col = call.has_flag("column");
if col {
length_col(engine_state, call, input)
} else {
length_row(call, input)
}
length_row(call, input)
}
fn examples(&self) -> Vec<Example> {
@ -55,32 +47,29 @@ impl Command for Length {
result: Some(Value::test_int(5)),
},
Example {
description: "Count the number of columns in a table",
example: "[{columnA: A0 columnB: B0}] | length -c",
description: "Count the number of rows in a table",
example: "[{a:1 b:2}, {a:2 b:3}] | length",
result: Some(Value::test_int(2)),
},
]
}
}
// this simulates calling input | columns | length
fn length_col(
engine_state: &EngineState,
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
length_row(
call,
getcol(engine_state, call.head, input)
.expect("getcol() should not fail used in column command"),
)
}
fn length_row(call: &Call, input: PipelineData) -> Result<PipelineData, ShellError> {
match input {
PipelineData::Value(Value::Nothing { .. }, ..) => {
Ok(Value::int(0, call.head).into_pipeline_data())
}
// I added this here because input_output_type() wasn't catching a record
// being sent in as input from echo. e.g. "echo {a:1 b:2} | length"
PipelineData::Value(Value::Record { span, .. }, ..) => {
Err(ShellError::OnlySupportsThisInputType {
exp_input_type: "list, and table".into(),
wrong_type: "record".into(),
dst_span: call.head,
src_span: span,
})
}
_ => {
let mut count: i64 = 0;
// Check for and propagate errors
@ -95,43 +84,6 @@ fn length_row(call: &Call, input: PipelineData) -> Result<PipelineData, ShellErr
}
}
fn getcol(
engine_state: &EngineState,
span: Span,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
match input {
PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::Value(
Value::List {
vals: input_vals,
span,
},
..,
) => {
let input_cols = get_columns(&input_vals);
Ok(input_cols
.into_iter()
.map(move |x| Value::String { val: x, span })
.into_pipeline_data(engine_state.ctrlc.clone()))
}
PipelineData::ListStream(stream, ..) => {
let v: Vec<_> = stream.into_iter().collect();
let input_cols = get_columns(&v);
Ok(input_cols
.into_iter()
.map(move |x| Value::String { val: x, span })
.into_pipeline_data(engine_state.ctrlc.clone()))
}
PipelineData::Value(..) | PipelineData::ExternalStream { .. } => {
let cols = vec![];
let vals = vec![];
Ok(Value::Record { cols, vals, span }.into_pipeline_data())
}
}
}
#[cfg(test)]
mod test {
use super::*;

5
crates/nu-command/tests/commands/first.rs
View File

@ -40,9 +40,10 @@ fn gets_first_row_when_no_amount_given() {
Playground::setup("first_test_3", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("caballeros.txt"), EmptyFile("arepas.clu")]);
let actual = nu!(cwd: dirs.test(), "ls | first | length");
// FIXME: We should probably change first to return a one row table instead of a record here
let actual = nu!(cwd: dirs.test(), "ls | first | values | length");
assert_eq!(actual.out, "1");
assert_eq!(actual.out, "4");
})
}

5
crates/nu-command/tests/commands/last.rs
View File

@ -33,9 +33,10 @@ fn gets_last_row_when_no_amount_given() {
Playground::setup("last_test_2", |dirs, sandbox| {
sandbox.with_files(vec![EmptyFile("caballeros.txt"), EmptyFile("arepas.clu")]);
let actual = nu!(cwd: dirs.test(), "ls | last | length");
// FIXME: We should probably change last to return a one row table instead of a record here
let actual = nu!(cwd: dirs.test(), "ls | last | values | length");
assert_eq!(actual.out, "1");
assert_eq!(actual.out, "4");
})
}

11
crates/nu-command/tests/commands/length.rs
View File

@ -2,14 +2,21 @@ use nu_test_support::nu;
#[test]
fn length_columns_in_cal_table() {
let actual = nu!("cal | length -c");
let actual = nu!("cal | columns | length");
assert_eq!(actual.out, "7");
}
#[test]
fn length_columns_no_rows() {
let actual = nu!("echo [] | length -c");
let actual = nu!("echo [] | length");
assert_eq!(actual.out, "0");
}
#[test]
fn length_fails_on_echo_record() {
let actual = nu!("echo {a:1 b:2} | length");
assert_eq!(actual.err.contains("only_supports_this_input_type"), true);
}

2
src/tests/test_table_operations.rs
View File

@ -264,7 +264,7 @@ fn update_will_insert() -> TestResult {
#[test]
fn length_for_columns() -> TestResult {
run_test(
r#"[[name,age,grade]; [bill,20,a] [a b c]] | length -c"#,
r#"[[name,age,grade]; [bill,20,a] [a b c]] | columns | length"#,
"3",
)
}