From 04fcbad12f99e79c642936dd511584a56586b176 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 11:39:15 +0200 Subject: [PATCH 1/7] allow "loose" detection of tables --- src/nu/value.rs | 16 ++++++++++------ src/ui.rs | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/nu/value.rs b/src/nu/value.rs index e1cbdcb..15dcd64 100644 --- a/src/nu/value.rs +++ b/src/nu/value.rs @@ -125,7 +125,7 @@ pub(crate) fn mutate_value_cell( Some(res) } -pub(crate) fn is_table(value: &Value) -> Table { +pub(crate) fn is_table(value: &Value, loose: bool) -> Table { match value { Value::List { vals, .. } => { if vals.is_empty() { @@ -145,6 +145,10 @@ pub(crate) fn is_table(value: &Value) -> Table { }; } + if loose { + return Table::IsValid; + } + // check the number of columns for each row let n = rows[0].keys().len(); for (i, row) in rows.iter().skip(1).enumerate() { @@ -227,7 +231,7 @@ pub(crate) fn is_table(value: &Value) -> Table { /// ``` // WARNING: some _unwraps_ haven't been proven to be safe in this function pub(crate) fn transpose(value: &Value) -> Value { - if matches!(is_table(value), Table::IsValid) { + if matches!(is_table(value, false), Table::IsValid) { let value_rows = match value { Value::List { vals, .. } => vals, _ => return value.clone(), @@ -540,7 +544,7 @@ mod tests { table_with_number_colum, ] { assert_eq!( - is_table(&table), + is_table(&table, false), Table::IsValid, "{} should be a table", default_value_repr(&table) @@ -607,15 +611,15 @@ mod tests { not_a_table_row_invalid_key, ] { assert_eq!( - is_table(¬_a_table), + is_table(¬_a_table, false), expected, "{} should not be a table", default_value_repr(¬_a_table) ); } - assert_eq!(is_table(&Value::test_int(0)), Table::NotAList); - assert_eq!(is_table(&Value::test_list(vec![])), Table::Empty); + assert_eq!(is_table(&Value::test_int(0), false), Table::NotAList); + assert_eq!(is_table(&Value::test_list(vec![]), false), Table::Empty); } #[test] diff --git a/src/ui.rs b/src/ui.rs index 11516ab..3f9acf2 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -238,7 +238,7 @@ fn render_data(frame: &mut Frame, app: &mut App) { let value = app.value_under_cursor(Some(CellPath { members: data_path })); - let table_type = is_table(&value); + let table_type = is_table(&value, false); let is_a_table = matches!(table_type, crate::nu::value::Table::IsValid); let mut data_frame_height = if config.show_cell_path { From 73aef28d7566a0478cee0779132c06a2c8f2833d Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 11:50:37 +0200 Subject: [PATCH 2/7] don't check types for "loosy" tables --- src/nu/value.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/nu/value.rs b/src/nu/value.rs index 15dcd64..e574d0a 100644 --- a/src/nu/value.rs +++ b/src/nu/value.rs @@ -145,10 +145,6 @@ pub(crate) fn is_table(value: &Value, loose: bool) -> Table { }; } - if loose { - return Table::IsValid; - } - // check the number of columns for each row let n = rows[0].keys().len(); for (i, row) in rows.iter().skip(1).enumerate() { @@ -168,6 +164,9 @@ pub(crate) fn is_table(value: &Value, loose: bool) -> Table { Some(v) => match ty { Type::Nothing => ty = v, _ => { + if loose { + continue; + } if !matches!(v, Type::Nothing) { if v.is_numeric() && ty.is_numeric() { } else if (!v.is_numeric() && ty.is_numeric()) From 62eee55c6af465cd98ec70ce69b705bae7e74c54 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 11:51:32 +0200 Subject: [PATCH 3/7] add tests for "loosy" tables --- src/nu/value.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/nu/value.rs b/src/nu/value.rs index e574d0a..7e0b150 100644 --- a/src/nu/value.rs +++ b/src/nu/value.rs @@ -548,6 +548,13 @@ mod tests { "{} should be a table", default_value_repr(&table) ); + + assert_eq!( + is_table(&table, true), + Table::IsValid, + "{} should be a loose table", + default_value_repr(&table) + ); } let not_a_table_missing_field = ( @@ -617,6 +624,26 @@ mod tests { ); } + let loosy_table_incompatible_types = Value::test_list(vec![ + Value::test_record(record! { + "a" => Value::test_string("a"), + "b" => Value::test_int(1), + }), + Value::test_record(record! { + "a" => Value::test_string("a"), + "b" => Value::test_list(vec![Value::test_int(1)]), + }), + ]); + + for loosy_table in [loosy_table_incompatible_types] { + assert_eq!( + is_table(&loosy_table, true), + Table::IsValid, + "{} should be a loose table", + default_value_repr(&loosy_table) + ); + } + assert_eq!(is_table(&Value::test_int(0), false), Table::NotAList); assert_eq!(is_table(&Value::test_list(vec![]), false), Table::Empty); } From 556797aafbb41dfefec756b56206da1cfbedd471 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 11:54:19 +0200 Subject: [PATCH 4/7] add `$.strict_tables` to the config --- examples/config/default.nuon | 1 + src/config/mod.rs | 7 +++++++ src/ui.rs | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/examples/config/default.nuon b/examples/config/default.nuon index 2895c62..8da13c4 100644 --- a/examples/config/default.nuon +++ b/examples/config/default.nuon @@ -6,6 +6,7 @@ margin: 10, # the number of lines to keep between the cursor and the top / bottom number: false, # show line numbers relativenumber: false, # show line numbers, relative to the current one (overrides number) + strict_tables: false, # only render tables when all rows have the same keys with the same types # "reset" is used instead of "black" in a dark terminal because, when the terminal is actually # black, "black" is not really black which is ugly, whereas "reset" is really black. diff --git a/src/config/mod.rs b/src/config/mod.rs index 24a7482..9e38357 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -150,6 +150,7 @@ pub struct Config { pub number: bool, pub relativenumber: bool, pub show_hints: bool, + pub strict_tables: bool, } impl Default for Config { @@ -164,6 +165,7 @@ impl Default for Config { number: false, relativenumber: false, show_hints: true, + strict_tables: false, colors: ColorConfig { normal: TableRowColorConfig { name: BgFgColorConfig { @@ -304,6 +306,11 @@ impl Config { config.show_hints = val } } + "strict_tables" => { + if let Some(val) = try_bool(value, &["strict_tables"])? { + config.strict_tables = val + } + } "colors" => { let cell = follow_cell_path(value, &["colors"]).unwrap(); let columns = match &cell { diff --git a/src/ui.rs b/src/ui.rs index 3f9acf2..1e19cd6 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -238,7 +238,7 @@ fn render_data(frame: &mut Frame, app: &mut App) { let value = app.value_under_cursor(Some(CellPath { members: data_path })); - let table_type = is_table(&value, false); + let table_type = is_table(&value, !config.strict_tables); let is_a_table = matches!(table_type, crate::nu::value::Table::IsValid); let mut data_frame_height = if config.show_cell_path { From 6cb7a7c5777259803fb117a3d38c0710f75ccf19 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 12:08:05 +0200 Subject: [PATCH 5/7] show the type as "any" if inconsistent in column --- src/ui.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index 1e19cd6..97c88b9 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -191,7 +191,7 @@ fn repr_data(data: &Value) -> Vec { /// /// > see the tests for detailed examples fn repr_table(table: &[Record]) -> (Vec, Vec, Vec>) { - let mut shapes = vec![Type::Nothing; table[0].len()]; + let mut shapes = vec![Type::Any; table[0].len()]; let mut rows = vec![vec![]; table.len()]; @@ -202,7 +202,12 @@ fn repr_table(table: &[Record]) -> (Vec, Vec, Vec>) let cell_type = val.get_type(); if !matches!(cell_type, Type::Nothing) { - if shapes[j].is_numeric() && cell_type.is_numeric() && (shapes[j] != cell_type) { + if shapes[j] != Type::Any && shapes[j] != cell_type { + shapes[j] = Type::Any; + } else if shapes[j].is_numeric() + && cell_type.is_numeric() + && (shapes[j] != cell_type) + { shapes[j] = Type::Number; } else { shapes[j] = cell_type; From e76c99cf1e43184ccac3169242bc38a015bf2e59 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 12:13:35 +0200 Subject: [PATCH 6/7] add a test for rendering "loose" tables --- src/ui.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/ui.rs b/src/ui.rs index 97c88b9..250183e 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -943,4 +943,26 @@ mod tests { assert_eq!(repr_table(&table), expected); } + + #[test] + fn repr_loose_table_with_mixed_types() { + let table = vec![ + record! { + "a" => Value::test_string("x"), + "b" => Value::test_int(1), + }, + record! { + "a" => Value::test_string("y"), + "b" => Value::test_string("z"), + }, + ]; + + let expected = ( + vec!["a".into(), "b".into()], + vec!["string".into(), "any".into()], + vec![vec!["x".into(), "1".into()], vec!["y".into(), "z".into()]], + ); + + assert_eq!(repr_table(&table), expected); + } } From 0541b0e339cd0aed2a8988191614a17f42cd9d70 Mon Sep 17 00:00:00 2001 From: amtoine Date: Sun, 14 Jul 2024 12:26:36 +0200 Subject: [PATCH 7/7] fix --- src/ui.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index 250183e..20cc2d4 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -191,7 +191,7 @@ fn repr_data(data: &Value) -> Vec { /// /// > see the tests for detailed examples fn repr_table(table: &[Record]) -> (Vec, Vec, Vec>) { - let mut shapes = vec![Type::Any; table[0].len()]; + let mut shapes = vec![Type::Nothing; table[0].len()]; let mut rows = vec![vec![]; table.len()]; @@ -201,17 +201,16 @@ fn repr_table(table: &[Record]) -> (Vec, Vec, Vec>) let val = row.get(col).unwrap(); let cell_type = val.get_type(); - if !matches!(cell_type, Type::Nothing) { - if shapes[j] != Type::Any && shapes[j] != cell_type { - shapes[j] = Type::Any; - } else if shapes[j].is_numeric() - && cell_type.is_numeric() - && (shapes[j] != cell_type) - { - shapes[j] = Type::Number; - } else { - shapes[j] = cell_type; - } + + if shapes[j].is_numeric() && cell_type.is_numeric() && (shapes[j] != cell_type) { + shapes[j] = Type::Number; + } else if shapes[j] == Type::Nothing && cell_type != Type::Nothing { + shapes[j] = cell_type; + } else if shapes[j] != Type::Nothing && cell_type == Type::Nothing { + } else if shapes[j] != cell_type { + shapes[j] = Type::Any; + } else { + shapes[j] = cell_type; } rows[i].push(repr_value(val).data);