From 6eb2c94209af69df0cf2e39b7f0624ea2300d7bb Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Mon, 20 Jul 2020 13:31:58 -0400 Subject: [PATCH] Add flag for case-insensitive sort-by (#2225) * Add flag for case-insensitive sort-by * Fix test names * Fix documentation comments --- crates/nu-cli/src/commands/math/median.rs | 2 +- crates/nu-cli/src/commands/math/mode.rs | 2 +- crates/nu-cli/src/commands/sort_by.rs | 62 ++++++++++++++++-- crates/nu-cli/tests/commands/sort_by.rs | 68 ++++++++++++++++++++ crates/nu-protocol/src/value.rs | 5 ++ docs/commands/sort-by.md | 68 ++++++++++++++++++++ tests/fixtures/formats/sample-ls-output.json | 1 + 7 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/formats/sample-ls-output.json diff --git a/crates/nu-cli/src/commands/math/median.rs b/crates/nu-cli/src/commands/math/median.rs index 36f223ba1..4bd8aa703 100644 --- a/crates/nu-cli/src/commands/math/median.rs +++ b/crates/nu-cli/src/commands/math/median.rs @@ -73,7 +73,7 @@ pub fn median(values: &[Value], name: &Tag) -> Result { sorted.push(item.clone()); } - crate::commands::sort_by::sort(&mut sorted, &[], name)?; + crate::commands::sort_by::sort(&mut sorted, &[], name, false)?; match take { Pick::Median => { diff --git a/crates/nu-cli/src/commands/math/mode.rs b/crates/nu-cli/src/commands/math/mode.rs index 2c894f7f9..010bf19a0 100644 --- a/crates/nu-cli/src/commands/math/mode.rs +++ b/crates/nu-cli/src/commands/math/mode.rs @@ -77,7 +77,7 @@ pub fn mode(values: &[Value], name: &Tag) -> Result { } } - crate::commands::sort_by::sort(&mut modes, &[], name)?; + crate::commands::sort_by::sort(&mut modes, &[], name, false)?; Ok(UntaggedValue::Table(modes).into_value(name)) } diff --git a/crates/nu-cli/src/commands/sort_by.rs b/crates/nu-cli/src/commands/sort_by.rs index 37a0a05a9..869853f43 100644 --- a/crates/nu-cli/src/commands/sort_by.rs +++ b/crates/nu-cli/src/commands/sort_by.rs @@ -11,6 +11,7 @@ pub struct SortBy; #[derive(Deserialize)] pub struct SortByArgs { rest: Vec>, + insensitive: bool, } #[async_trait] @@ -20,7 +21,13 @@ impl WholeStreamCommand for SortBy { } fn signature(&self) -> Signature { - Signature::build("sort-by").rest(SyntaxShape::String, "the column(s) to sort by") + Signature::build("sort-by") + .switch( + "insensitive", + "Sort string-based columns case insensitively", + Some('i'), + ) + .rest(SyntaxShape::String, "the column(s) to sort by") } fn usage(&self) -> &str { @@ -57,6 +64,24 @@ impl WholeStreamCommand for SortBy { example: "ls | sort-by type size", result: None, }, + Example { + description: "Sort strings (case sensitive)", + example: "echo [airplane Truck Car] | sort-by", + result: Some(vec![ + UntaggedValue::string("Car").into(), + UntaggedValue::string("Truck").into(), + UntaggedValue::string("airplane").into(), + ]), + }, + Example { + description: "Sort strings (case insensitive)", + example: "echo [airplane Truck Car] | sort-by -i", + result: Some(vec![ + UntaggedValue::string("airplane").into(), + UntaggedValue::string("Car").into(), + UntaggedValue::string("Truck").into(), + ]), + }, ] } } @@ -68,10 +93,10 @@ async fn sort_by( let registry = registry.clone(); let tag = args.call_info.name_tag.clone(); - let (SortByArgs { rest }, mut input) = args.process(®istry).await?; + let (SortByArgs { rest, insensitive }, mut input) = args.process(®istry).await?; let mut vec = input.drain_vec().await; - sort(&mut vec, &rest, &tag)?; + sort(&mut vec, &rest, &tag, insensitive)?; Ok(futures::stream::iter(vec.into_iter()).to_output_stream()) } @@ -80,6 +105,7 @@ pub fn sort( vec: &mut [Value], keys: &[Tagged], tag: impl Into, + insensitive: bool, ) -> Result<(), ShellError> { let tag = tag.into(); @@ -107,12 +133,38 @@ pub fn sort( value: UntaggedValue::Primitive(_), .. } => { - vec.sort_by(|a, b| coerce_compare(a, b).expect("Unimplemented BUG: What about primitives that don't have an order defined?").compare()); + let should_sort_case_insensitively = insensitive && vec.iter().all(|x| x.is_string()); + + vec.sort_by(|a, b| { + if should_sort_case_insensitively { + let lowercase_a_string = a.expect_string().to_ascii_lowercase(); + let lowercase_b_string = b.expect_string().to_ascii_lowercase(); + + lowercase_a_string.cmp(&lowercase_b_string) + } else { + coerce_compare(a, b).expect("Unimplemented BUG: What about primitives that don't have an order defined?").compare() + } + }); } _ => { let calc_key = |item: &Value| { keys.iter() - .map(|f| get_data_by_key(item, f.borrow_spanned())) + .map(|f| { + let mut value_option = get_data_by_key(item, f.borrow_spanned()); + + if insensitive { + if let Some(value) = &value_option { + if let Ok(string_value) = value.as_string() { + value_option = Some( + UntaggedValue::string(string_value.to_ascii_lowercase()) + .into_value(value.tag.clone()), + ) + } + } + } + + value_option + }) .collect::>>() }; vec.sort_by_cached_key(calc_key); diff --git a/crates/nu-cli/tests/commands/sort_by.rs b/crates/nu-cli/tests/commands/sort_by.rs index 15bb44cd0..4aa22524e 100644 --- a/crates/nu-cli/tests/commands/sort_by.rs +++ b/crates/nu-cli/tests/commands/sort_by.rs @@ -62,3 +62,71 @@ fn sort_primitive_values() { assert_eq!(actual.out, "authors = [\"The Nu Project Contributors\"]"); } + +#[test] +fn ls_sort_by_name_sensitive() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + open sample-ls-output.json + | sort-by name + | select name + | to json + "# + )); + + let json_output = r#"[{"name":"B.txt"},{"name":"C"},{"name":"a.txt"}]"#; + + assert_eq!(actual.out, json_output); +} + +#[test] +fn ls_sort_by_name_insensitive() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + open sample-ls-output.json + | sort-by -i name + | select name + | to json + "# + )); + + let json_output = r#"[{"name":"a.txt"},{"name":"B.txt"},{"name":"C"}]"#; + + assert_eq!(actual.out, json_output); +} + +#[test] +fn ls_sort_by_type_name_sensitive() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + open sample-ls-output.json + | sort-by type name + | select name type + | to json + "# + )); + + let json_output = r#"[{"name":"C","type":"Dir"},{"name":"B.txt","type":"File"},{"name":"a.txt","type":"File"}]"#; + + assert_eq!(actual.out, json_output); +} + +#[test] +fn ls_sort_by_type_name_insensitive() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + open sample-ls-output.json + | sort-by -i type name + | select name type + | to json + "# + )); + + let json_output = r#"[{"name":"C","type":"Dir"},{"name":"a.txt","type":"File"},{"name":"B.txt","type":"File"}]"#; + + assert_eq!(actual.out, json_output); +} diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index c2e7964f1..48e398d4a 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -85,6 +85,11 @@ impl UntaggedValue { matches!(self, UntaggedValue::Table(_)) } + /// Returns true if this value represents a string + pub fn is_string(&self) -> bool { + matches!(self, UntaggedValue::Primitive(Primitive::String(_))) + } + /// Returns true if the value represents something other than Nothing pub fn is_some(&self) -> bool { !self.is_none() diff --git a/docs/commands/sort-by.md b/docs/commands/sort-by.md index da7911d82..82dfcdaaa 100644 --- a/docs/commands/sort-by.md +++ b/docs/commands/sort-by.md @@ -5,6 +5,10 @@ The `sort-by` command sorts the table being displayed in the terminal by a chose `sort-by` takes multiple arguments (being the names of columns) sorting by each argument in order. +## Flags + +* `-i`, `--insensitive`: Sort string-based columns case insensitively + ## Examples ```shell @@ -53,3 +57,67 @@ The `sort-by` command sorts the table being displayed in the terminal by a chose 7 │ az │ File │ │ 18 B │ 5 minutes ago │ 5 minutes ago ━━━┷━━━━━━┷━━━━━━┷━━━━━━━━━━┷━━━━━━━━┷━━━━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━━ ``` + +Within the Nushell repository... + +```shell +> ls | sort-by --insensitive name +────┬────────────────────┬──────┬──────────┬────────────── + # │ name │ type │ size │ modified +────┼────────────────────┼──────┼──────────┼────────────── + 0 │ assets │ Dir │ 128 B │ 6 months ago + 1 │ build.rs │ File │ 78 B │ 5 months ago + 2 │ Cargo.lock │ File │ 118.3 KB │ 1 hour ago + 3 │ Cargo.toml │ File │ 5.5 KB │ 1 hour ago + 4 │ CODE_OF_CONDUCT.md │ File │ 3.4 KB │ 1 hour ago + 5 │ CONTRIBUTING.md │ File │ 1.3 KB │ 1 hour ago + 6 │ crates │ Dir │ 832 B │ 1 hour ago + 7 │ debian │ Dir │ 352 B │ 6 months ago + 8 │ docker │ Dir │ 288 B │ 4 months ago + 9 │ docs │ Dir │ 192 B │ 1 hour ago + 10 │ features.toml │ File │ 632 B │ 5 months ago + 11 │ images │ Dir │ 160 B │ 6 months ago + 12 │ LICENSE │ File │ 1.1 KB │ 4 months ago + 13 │ Makefile.toml │ File │ 449 B │ 6 months ago + 14 │ README.build.txt │ File │ 192 B │ 1 hour ago + 15 │ README.md │ File │ 16.0 KB │ 1 hour ago + 16 │ rustfmt.toml │ File │ 16 B │ 6 months ago + 17 │ src │ Dir │ 128 B │ 1 week ago + 18 │ target │ Dir │ 160 B │ 1 day ago + 19 │ tests │ Dir │ 192 B │ 4 months ago + 20 │ TODO.md │ File │ 0 B │ 1 week ago + 21 │ wix │ Dir │ 128 B │ 1 hour ago +────┴────────────────────┴──────┴──────────┴────────────── +``` + +Within the Nushell repository... + +```shell +> ls | sort-by --insensitive type name +────┬────────────────────┬──────┬──────────┬────────────── + # │ name │ type │ size │ modified +────┼────────────────────┼──────┼──────────┼────────────── + 0 │ assets │ Dir │ 128 B │ 6 months ago + 1 │ crates │ Dir │ 832 B │ 1 hour ago + 2 │ debian │ Dir │ 352 B │ 6 months ago + 3 │ docker │ Dir │ 288 B │ 4 months ago + 4 │ docs │ Dir │ 192 B │ 1 hour ago + 5 │ images │ Dir │ 160 B │ 6 months ago + 6 │ src │ Dir │ 128 B │ 1 week ago + 7 │ target │ Dir │ 160 B │ 1 day ago + 8 │ tests │ Dir │ 192 B │ 4 months ago + 9 │ wix │ Dir │ 128 B │ 1 hour ago + 10 │ build.rs │ File │ 78 B │ 5 months ago + 11 │ Cargo.lock │ File │ 118.3 KB │ 1 hour ago + 12 │ Cargo.toml │ File │ 5.5 KB │ 1 hour ago + 13 │ CODE_OF_CONDUCT.md │ File │ 3.4 KB │ 1 hour ago + 14 │ CONTRIBUTING.md │ File │ 1.3 KB │ 1 hour ago + 15 │ features.toml │ File │ 632 B │ 5 months ago + 16 │ LICENSE │ File │ 1.1 KB │ 4 months ago + 17 │ Makefile.toml │ File │ 449 B │ 6 months ago + 18 │ README.build.txt │ File │ 192 B │ 1 hour ago + 19 │ README.md │ File │ 16.0 KB │ 1 hour ago + 20 │ rustfmt.toml │ File │ 16 B │ 6 months ago + 21 │ TODO.md │ File │ 0 B │ 1 week ago +────┴────────────────────┴──────┴──────────┴────────────── +``` diff --git a/tests/fixtures/formats/sample-ls-output.json b/tests/fixtures/formats/sample-ls-output.json new file mode 100644 index 000000000..4636f0353 --- /dev/null +++ b/tests/fixtures/formats/sample-ls-output.json @@ -0,0 +1 @@ +[{"name":"a.txt","type":"File","size":3444,"modified":"2020-07-1918:26:30.560716967UTC"},{"name":"B.txt","type":"File","size":1341,"modified":"2020-07-1918:26:30.561021953UTC"},{"name":"C","type":"Dir","size":118253,"modified":"2020-07-1918:26:30.562092480UTC"}]