Display empty records and lists (#7925)

# Description

Fix some issues related to #7444 
1. Empty lists and records are now displayed as a small notice in a box:

![image](https://user-images.githubusercontent.com/17511668/215832023-3f8d743a-2899-416f-9109-7876ad2bbedf.png)

![image](https://user-images.githubusercontent.com/17511668/215832273-c737b8a4-af33-4c16-8dd3-bd4f0fd19b5a.png)
2. Empty records are now correctly displayed if inside of another record
list or table:

![image](https://user-images.githubusercontent.com/17511668/215832597-00f0cebc-a3b6-4ce8-8373-a9340d4c7020.png)

![image](https://user-images.githubusercontent.com/17511668/215832540-ab0e2a14-b8f6-4f47-976c-42003b622ef6.png)
3. Fixed inconsistent coloring of empty list placeholder inside of
lists/tables:

![image](https://user-images.githubusercontent.com/17511668/215832924-813ffe17-e04e-4301-97c3-1bdbccf1825c.png)

![image](https://user-images.githubusercontent.com/17511668/215832963-4765c4cf-3036-4bcc-81e1-ced941fa47cb.png)


# User-Facing Changes

`table` command now displays empty records and lists like a table with
text and correctly displays empty records inside tables and lists.

New behavior of displaying empty lists and records can be disabled using
`table.show_empty` config option.

# 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 -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# 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:
Artemiy 2023-02-22 19:18:33 +03:00 committed by GitHub
parent 0ab6b66d8f
commit e389e51b2b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 256 additions and 117 deletions

View file

@ -118,7 +118,7 @@ impl<'a> StyleComputer<'a> {
// Used only by the `table` command. // Used only by the `table` command.
pub fn style_primitive(&self, value: &Value) -> TextStyle { pub fn style_primitive(&self, value: &Value) -> TextStyle {
let s = self.compute(&value.get_type().to_string(), value); let s = self.compute(&value.get_type().get_non_specified_string(), value);
match *value { match *value {
Value::Bool { .. } => TextStyle::with_style(AlignmentHorizontal::Left, s), Value::Bool { .. } => TextStyle::with_style(AlignmentHorizontal::Left, s),

View file

@ -55,7 +55,7 @@ impl Command for Table {
vec!["display", "render"] vec!["display", "render"]
} }
fn signature(&self) -> nu_protocol::Signature { fn signature(&self) -> Signature {
Signature::build("table") Signature::build("table")
.input_output_types(vec![(Type::Any, Type::Any)]) .input_output_types(vec![(Type::Any, Type::Any)])
// TODO: make this more precise: what turns into string and what into raw stream // TODO: make this more precise: what turns into string and what into raw stream
@ -276,61 +276,18 @@ fn handle_table_command(
ctrlc, ctrlc,
metadata, metadata,
), ),
PipelineData::Value(Value::Record { cols, vals, span }, ..) => { PipelineData::Value(Value::Record { cols, vals, span }, ..) => handle_record(
// Create a StyleComputer to compute styles for each value in the table. cols,
let style_computer = &StyleComputer::from_config(engine_state, stack); vals,
let result = match table_view { span,
TableView::General => build_general_table2( engine_state,
style_computer, stack,
cols, call,
vals, table_view,
ctrlc.clone(), term_width,
config, ctrlc,
term_width, config,
), ),
TableView::Expanded {
limit,
flatten,
flatten_separator,
} => {
let sep = flatten_separator.as_deref().unwrap_or(" ");
build_expanded_table(
cols,
vals,
span,
ctrlc.clone(),
config,
style_computer,
term_width,
limit,
flatten,
sep,
)
}
TableView::Collapsed => {
build_collapsed_table(style_computer, cols, vals, config, term_width)
}
}?;
let result = strip_output_color(result, config);
let result = result.unwrap_or_else(|| {
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
"".into()
} else {
// assume this failed because the table was too wide
// TODO: more robust error classification
format!("Couldn't fit table into {term_width} columns!")
}
});
let val = Value::String {
val: result,
span: call.head,
};
Ok(val.into_pipeline_data())
}
PipelineData::Value(Value::LazyRecord { val, .. }, ..) => { PipelineData::Value(Value::LazyRecord { val, .. }, ..) => {
let collected = val.collect()?.into_pipeline_data(); let collected = val.collect()?.into_pipeline_data();
handle_table_command( handle_table_command(
@ -527,32 +484,39 @@ fn build_expanded_table(
} }
} }
Value::Record { cols, vals, span } => { Value::Record { cols, vals, span } => {
let result = build_expanded_table( if cols.is_empty() {
cols.clone(), // Like list case return styled string instead of empty value
vals.clone(), let value = Value::Record { cols, vals, span };
span, let text = value_to_styled_string(&value, config, style_computer).0;
ctrlc.clone(), wrap_text(&text, value_width, config)
config, } else {
style_computer, let result = build_expanded_table(
value_width, cols.clone(),
deep, vals.clone(),
flatten, span,
flatten_sep, ctrlc.clone(),
)?; config,
style_computer,
value_width,
deep,
flatten,
flatten_sep,
)?;
match result { match result {
Some(result) => { Some(result) => {
is_expanded = true; is_expanded = true;
result result
} }
None => { None => {
let failed_value = value_to_styled_string( let failed_value = value_to_styled_string(
&Value::Record { cols, vals, span }, &Value::Record { cols, vals, span },
config, config,
style_computer, style_computer,
); );
wrap_text(&failed_value.0, value_width, config) wrap_text(&failed_value.0, value_width, config)
}
} }
} }
} }
@ -607,6 +571,79 @@ fn build_expanded_table(
Ok(table) Ok(table)
} }
#[allow(clippy::too_many_arguments)]
fn handle_record(
cols: Vec<String>,
vals: Vec<Value>,
span: Span,
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
table_view: TableView,
term_width: usize,
ctrlc: Option<Arc<AtomicBool>>,
config: &Config,
) -> Result<PipelineData, ShellError> {
// Create a StyleComputer to compute styles for each value in the table.
let style_computer = &StyleComputer::from_config(engine_state, stack);
let result = if cols.is_empty() {
create_empty_placeholder("record", term_width, engine_state, stack)
} else {
let result = match table_view {
TableView::General => build_general_table2(
style_computer,
cols,
vals,
ctrlc.clone(),
config,
term_width,
),
TableView::Expanded {
limit,
flatten,
flatten_separator,
} => {
let sep = flatten_separator.as_deref().unwrap_or(" ");
build_expanded_table(
cols,
vals,
span,
ctrlc.clone(),
config,
style_computer,
term_width,
limit,
flatten,
sep,
)
}
TableView::Collapsed => {
build_collapsed_table(style_computer, cols, vals, config, term_width)
}
}?;
let result = strip_output_color(result, config);
result.unwrap_or_else(|| {
if nu_utils::ctrl_c::was_pressed(&ctrlc) {
"".into()
} else {
// assume this failed because the table was too wide
// TODO: more robust error classification
format!("Couldn't fit table into {term_width} columns!")
}
})
};
let val = Value::String {
val: result,
span: call.head,
};
Ok(val.into_pipeline_data())
}
fn handle_row_stream( fn handle_row_stream(
engine_state: &EngineState, engine_state: &EngineState,
stack: &mut Stack, stack: &mut Stack,
@ -721,18 +758,18 @@ fn handle_row_stream(
Ok(PipelineData::ExternalStream { Ok(PipelineData::ExternalStream {
stdout: Some(RawStream::new( stdout: Some(RawStream::new(
Box::new(PagingTableCreator { Box::new(PagingTableCreator::new(
row_offset,
// These are passed in as a way to have PagingTable create StyleComputers
// for the values it outputs. Because engine_state is passed in, config doesn't need to.
engine_state: engine_state.clone(),
stack: stack.clone(),
ctrlc: ctrlc.clone(),
head, head,
stream, stream,
// These are passed in as a way to have PagingTable create StyleComputers
// for the values it outputs. Because engine_state is passed in, config doesn't need to.
engine_state.clone(),
stack.clone(),
ctrlc.clone(),
row_offset,
width_param, width_param,
view: table_view, table_view,
}), )),
ctrlc, ctrlc,
head, head,
None, None,
@ -1522,9 +1559,36 @@ struct PagingTableCreator {
row_offset: usize, row_offset: usize,
width_param: Option<i64>, width_param: Option<i64>,
view: TableView, view: TableView,
elements_displayed: usize,
reached_end: bool,
} }
impl PagingTableCreator { impl PagingTableCreator {
#[allow(clippy::too_many_arguments)]
fn new(
head: Span,
stream: ListStream,
engine_state: EngineState,
stack: Stack,
ctrlc: Option<Arc<AtomicBool>>,
row_offset: usize,
width_param: Option<i64>,
view: TableView,
) -> Self {
PagingTableCreator {
head,
stream,
engine_state,
stack,
ctrlc,
row_offset,
width_param,
view,
elements_displayed: 0,
reached_end: false,
}
}
fn build_extended( fn build_extended(
&mut self, &mut self,
batch: &[Value], batch: &[Value],
@ -1665,6 +1729,7 @@ impl Iterator for PagingTableCreator {
let start_time = Instant::now(); let start_time = Instant::now();
let mut idx = 0; let mut idx = 0;
let mut reached_end = true;
// Pull from stream until time runs out or we have enough items // Pull from stream until time runs out or we have enough items
for item in self.stream.by_ref() { for item in self.stream.by_ref() {
@ -1673,10 +1738,12 @@ impl Iterator for PagingTableCreator {
// If we've been buffering over a second, go ahead and send out what we have so far // If we've been buffering over a second, go ahead and send out what we have so far
if (Instant::now() - start_time).as_secs() >= 1 { if (Instant::now() - start_time).as_secs() >= 1 {
reached_end = false;
break; break;
} }
if idx == STREAM_PAGE_SIZE { if idx == STREAM_PAGE_SIZE {
reached_end = false;
break; break;
} }
@ -1685,8 +1752,24 @@ impl Iterator for PagingTableCreator {
} }
} }
// Count how much elements were displayed and if end of stream was reached
self.elements_displayed += idx;
self.reached_end = self.reached_end || reached_end;
if batch.is_empty() { if batch.is_empty() {
return None; // If this iterator has not displayed a single entry and reached its end (no more elements
// or interrupted by ctrl+c) display as "empty list"
return if self.elements_displayed == 0 && self.reached_end {
// Increase elements_displayed by one so on next iteration next branch of this
// if else triggers and terminates stream
self.elements_displayed = 1;
let term_width = get_width_param(self.width_param);
let result =
create_empty_placeholder("list", term_width, &self.engine_state, &self.stack);
Some(Ok(result.into_bytes()))
} else {
None
};
} }
let table = match &self.view { let table = match &self.view {
@ -1729,17 +1812,17 @@ impl Iterator for PagingTableCreator {
fn load_theme_from_config(config: &Config) -> TableTheme { fn load_theme_from_config(config: &Config) -> TableTheme {
match config.table_mode.as_str() { match config.table_mode.as_str() {
"basic" => nu_table::TableTheme::basic(), "basic" => TableTheme::basic(),
"thin" => nu_table::TableTheme::thin(), "thin" => TableTheme::thin(),
"light" => nu_table::TableTheme::light(), "light" => TableTheme::light(),
"compact" => nu_table::TableTheme::compact(), "compact" => TableTheme::compact(),
"with_love" => nu_table::TableTheme::with_love(), "with_love" => TableTheme::with_love(),
"compact_double" => nu_table::TableTheme::compact_double(), "compact_double" => TableTheme::compact_double(),
"rounded" => nu_table::TableTheme::rounded(), "rounded" => TableTheme::rounded(),
"reinforced" => nu_table::TableTheme::reinforced(), "reinforced" => TableTheme::reinforced(),
"heavy" => nu_table::TableTheme::heavy(), "heavy" => TableTheme::heavy(),
"none" => nu_table::TableTheme::none(), "none" => TableTheme::none(),
_ => nu_table::TableTheme::rounded(), _ => TableTheme::rounded(),
} }
} }
@ -1849,6 +1932,31 @@ fn need_footer(config: &Config, count_records: u64) -> bool {
|| matches!(config.footer_mode, FooterMode::Always) || matches!(config.footer_mode, FooterMode::Always)
} }
fn create_empty_placeholder(
value_type_name: &str,
termwidth: usize,
engine_state: &EngineState,
stack: &Stack,
) -> String {
let config = engine_state.get_config();
if !config.table_show_empty {
return "".into();
}
let empty_info_string = format!("empty {}", value_type_name);
let cell = NuTable::create_cell(empty_info_string, TextStyle::default().dimmed());
let data = vec![vec![cell]];
let table = NuTable::new(data, (1, 1));
let style_computer = &StyleComputer::from_config(engine_state, stack);
let config = create_table_config(config, style_computer, 1, false, false, false);
table
.draw(config, termwidth)
.expect("Could not create empty table placeholder")
}
fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) { fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) {
match value { match value {
Value::Record { cols, vals, .. } => { Value::Record { cols, vals, .. } => {

View file

@ -55,11 +55,11 @@ fn find_with_string_search_with_string_not_found() {
let actual = nu!( let actual = nu!(
cwd: ".", pipeline( cwd: ".", pipeline(
r#" r#"
[moe larry curly] | find shemp [moe larry curly] | find shemp | is-empty
"# "#
)); ));
assert_eq!(actual.out, ""); assert_eq!(actual.out, "true");
} }
#[test] #[test]

View file

@ -5,11 +5,11 @@ fn splits_empty_path() {
let actual = nu!( let actual = nu!(
cwd: "tests", pipeline( cwd: "tests", pipeline(
r#" r#"
echo '' | path split echo '' | path split | is-empty
"# "#
)); ));
assert_eq!(actual.out, ""); assert_eq!(actual.out, "true");
} }
#[test] #[test]

View file

@ -8,7 +8,7 @@ fn better_empty_redirection() {
let actual = nu!( let actual = nu!(
cwd: "tests/fixtures/formats", pipeline( cwd: "tests/fixtures/formats", pipeline(
r#" r#"
ls | each { |it| nu --testbin cococo $it.name } ls | each { |it| nu --testbin cococo $it.name } | ignore
"# "#
)); ));

View file

@ -1380,10 +1380,8 @@ fn table_expande_with_no_header_internally_0() {
"│ │ │ │ │ highlight │ │ bg │ yellow │ │ │ │", "│ │ │ │ │ highlight │ │ bg │ yellow │ │ │ │",
"│ │ │ │ │ │ │ fg │ black │ │ │ │", "│ │ │ │ │ │ │ fg │ black │ │ │ │",
"│ │ │ │ │ │ ╰────┴────────╯ │ │ │", "│ │ │ │ │ │ ╰────┴────────╯ │ │ │",
"│ │ │ │ │ │ │ │ │", "│ │ │ │ │ status │ {record 0 fields} │ │ │",
"│ │ │ │ │ status │ │ │ │", "│ │ │ │ │ try │ {record 0 fields} │ │ │",
"│ │ │ │ │ │ │ │ │",
"│ │ │ │ │ try │ │ │ │",
"│ │ │ │ │ │ ╭──────────────────┬─────────╮ │ │ │", "│ │ │ │ │ │ ╭──────────────────┬─────────╮ │ │ │",
"│ │ │ │ │ table │ │ split_line │ #404040 │ │ │ │", "│ │ │ │ │ table │ │ split_line │ #404040 │ │ │ │",
"│ │ │ │ │ │ │ cursor │ true │ │ │ │", "│ │ │ │ │ │ │ cursor │ true │ │ │ │",
@ -1624,10 +1622,8 @@ fn table_expande_with_no_header_internally_1() {
"│ │ │ │ │ highlight │ │ bg │ yellow │ │ │ │", "│ │ │ │ │ highlight │ │ bg │ yellow │ │ │ │",
"│ │ │ │ │ │ │ fg │ black │ │ │ │", "│ │ │ │ │ │ │ fg │ black │ │ │ │",
"│ │ │ │ │ │ ╰────┴────────╯ │ │ │", "│ │ │ │ │ │ ╰────┴────────╯ │ │ │",
"│ │ │ │ │ │ │ │ │", "│ │ │ │ │ status │ {record 0 fields} │ │ │",
"│ │ │ │ │ status │ │ │ │", "│ │ │ │ │ try │ {record 0 fields} │ │ │",
"│ │ │ │ │ │ │ │ │",
"│ │ │ │ │ try │ │ │ │",
"│ │ │ │ │ │ ╭──────────────────┬─────────╮ │ │ │", "│ │ │ │ │ │ ╭──────────────────┬─────────╮ │ │ │",
"│ │ │ │ │ table │ │ split_line │ #404040 │ │ │ │", "│ │ │ │ │ table │ │ split_line │ #404040 │ │ │ │",
"│ │ │ │ │ │ │ cursor │ true │ │ │ │", "│ │ │ │ │ │ │ cursor │ true │ │ │ │",

View file

@ -65,6 +65,7 @@ pub struct Config {
pub external_completer: Option<usize>, pub external_completer: Option<usize>,
pub filesize_metric: bool, pub filesize_metric: bool,
pub table_mode: String, pub table_mode: String,
pub table_show_empty: bool,
pub use_ls_colors: bool, pub use_ls_colors: bool,
pub color_config: HashMap<String, Value>, pub color_config: HashMap<String, Value>,
pub use_grid_icons: bool, pub use_grid_icons: bool,
@ -106,6 +107,7 @@ impl Default for Config {
Config { Config {
filesize_metric: false, filesize_metric: false,
table_mode: "rounded".into(), table_mode: "rounded".into(),
table_show_empty: true,
external_completer: None, external_completer: None,
use_ls_colors: true, use_ls_colors: true,
color_config: HashMap::new(), color_config: HashMap::new(),
@ -885,6 +887,9 @@ impl Value {
} }
} }
} }
"show_empty" => {
try_bool!(cols, vals, index, span, table_show_empty)
}
x => { x => {
invalid_key!( invalid_key!(
cols, cols,

View file

@ -96,6 +96,35 @@ impl Type {
Type::Signature => SyntaxShape::Signature, Type::Signature => SyntaxShape::Signature,
} }
} }
/// Get a string representation, without inner type specification of lists,
/// tables and records (get `list` instead of `list<any>`
pub fn get_non_specified_string(&self) -> String {
match self {
Type::Block => String::from("block"),
Type::Closure => String::from("closure"),
Type::Bool => String::from("bool"),
Type::CellPath => String::from("cell path"),
Type::Date => String::from("date"),
Type::Duration => String::from("duration"),
Type::Filesize => String::from("filesize"),
Type::Float => String::from("float"),
Type::Int => String::from("int"),
Type::Range => String::from("range"),
Type::Record(_) => String::from("record"),
Type::Table(_) => String::from("table"),
Type::List(_) => String::from("list"),
Type::Nothing => String::from("nothing"),
Type::Number => String::from("number"),
Type::String => String::from("string"),
Type::ListStream => String::from("list stream"),
Type::Any => String::from("any"),
Type::Error => String::from("error"),
Type::Binary => String::from("binary"),
Type::Custom(_) => String::from("custom"),
Type::Signature => String::from("signature"),
}
}
} }
impl Display for Type { impl Display for Type {

View file

@ -187,6 +187,7 @@ let-env config = {
table: { table: {
mode: rounded # basic, compact, compact_double, light, thin, with_love, rounded, reinforced, heavy, none, other mode: rounded # basic, compact, compact_double, light, thin, with_love, rounded, reinforced, heavy, none, other
index_mode: always # "always" show indexes, "never" show indexes, "auto" = show indexes when a table has "index" column index_mode: always # "always" show indexes, "never" show indexes, "auto" = show indexes when a table has "index" column
show_empty: true # show 'empty list' and 'empty record' placeholders for command output
trim: { trim: {
methodology: wrapping # wrapping or truncating methodology: wrapping # wrapping or truncating
wrapping_try_keep_words: true # A strategy used by the 'wrapping' methodology wrapping_try_keep_words: true # A strategy used by the 'wrapping' methodology

View file

@ -299,11 +299,11 @@ fn run_custom_command_with_empty_rest() {
let actual = nu!( let actual = nu!(
cwd: ".", cwd: ".",
r#" r#"
def rest-me-with-empty-rest [...rest: string] { echo $rest }; rest-me-with-empty-rest def rest-me-with-empty-rest [...rest: string] { $rest }; rest-me-with-empty-rest | is-empty
"# "#
); );
assert_eq!(actual.out, r#""#); assert_eq!(actual.out, r#"true"#);
assert_eq!(actual.err, r#""#); assert_eq!(actual.err, r#""#);
} }