Rely on display_output hook for formatting values from evaluations (#14361)

# Description

I was reading through the documentation yesterday, when I stumbled upon
[this
section](https://www.nushell.sh/book/pipelines.html#behind-the-scenes)
explaining how command output is formatted using the `table` command. I
was surprised that this section didn't mention the `display_output`
hook, so I took a look in the code and was shocked to discovered that
the documentation was correct, and the `table` command _is_
automatically applied to printed pipelines.

This auto-tabling has two ramifications for the `display_output` hook:

1. The `table` command is called on the output of a pipeline after the
`display_output` has run, even if `display_output` contains the table
command. This means each pipeline output is roughly equivalent to the
following (using `ls` as an example):
    ```nushell
    ls | do $config.hooks.display_output | table
    ```
2. If `display_output` returns structured data, it will _still_ be
formatted through the table command.

This PR removes the auto-table when the `display_output` hook is set.
The auto-table made sense before `display_output` was introduced, but to
me, it now seems like unnecessary "automagic" which can be accomplished
using existing Nushell features.

This means that you can now pull back the curtain a bit, and replace
your `display_output` hook with an empty closure
(`$env.config.hooks.display_output = {||}`, setting it to null retains
the previous behavior) to see the values printed normally without the
table formatting. I think this is a good thing, and makes it easier to
understand Nushell fundamentals.

It is important to note that this PR does not change how `print` and
other commands (well, specifically only `watch`) print out values. They
continue to use `table` with no arguments, so changing your
config/`display_output` hook won't affect what `print`ing a value does.

Rel: [Discord
discussion](https://discord.com/channels/601130461678272522/615329862395101194/1307102690848931904)
(cc @dcarosone)

# User-Facing Changes

Pipelines are no longer automatically formatted using the `table`
command. Instead, the `display_output` hook is used to format pipeline
output. Most users should see no impact, as the default `display_output`
hook already uses the `table` command.

# Tests + Formatting

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting


Will update mentioned docs page to call out `display_output` hook.
This commit is contained in:
132ikl 2024-11-19 08:04:29 -05:00 committed by GitHub
parent 6e84ba182e
commit d69e131450
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 50 additions and 36 deletions

View File

@ -9,6 +9,8 @@ use nu_protocol::{
};
use std::sync::Arc;
use crate::util::print_pipeline;
#[derive(Default)]
pub struct EvaluateCommandsOpts {
pub table_mode: Option<Value>,
@ -93,7 +95,7 @@ pub fn evaluate_commands(
t_mode.coerce_str()?.parse().unwrap_or_default();
}
pipeline.print(engine_state, stack, no_newline, false)?;
print_pipeline(engine_state, stack, pipeline, no_newline)?;
info!("evaluate {}:{}:{}", file!(), line!(), column!());

View File

@ -1,4 +1,4 @@
use crate::util::eval_source;
use crate::util::{eval_source, print_pipeline};
use log::{info, trace};
use nu_engine::{convert_env_values, eval_block};
use nu_parser::parse;
@ -119,7 +119,7 @@ pub fn evaluate_file(
};
// Print the pipeline output of the last command of the file.
pipeline.print(engine_state, stack, true, false)?;
print_pipeline(engine_state, stack, pipeline, true)?;
// Invoke the main command with arguments.
// Arguments with whitespaces are quoted, thus can be safely concatenated by whitespace.

View File

@ -65,8 +65,12 @@ Since this command has no output, there is no point in piping it with other comm
arg.into_pipeline_data()
.print_raw(engine_state, no_newline, to_stderr)?;
} else {
arg.into_pipeline_data()
.print(engine_state, stack, no_newline, to_stderr)?;
arg.into_pipeline_data().print_table(
engine_state,
stack,
no_newline,
to_stderr,
)?;
}
}
} else if !input.is_nothing() {
@ -78,7 +82,7 @@ Since this command has no output, there is no point in piping it with other comm
if raw {
input.print_raw(engine_state, no_newline, to_stderr)?;
} else {
input.print(engine_state, stack, no_newline, to_stderr)?;
input.print_table(engine_state, stack, no_newline, to_stderr)?;
}
}

View File

@ -201,6 +201,35 @@ fn gather_env_vars(
}
}
/// Print a pipeline with formatting applied based on display_output hook.
///
/// This function should be preferred when printing values resulting from a completed evaluation.
/// For values printed as part of a command's execution, such as values printed by the `print` command,
/// the `PipelineData::print_table` function should be preferred instead as it is not config-dependent.
///
/// `no_newline` controls if we need to attach newline character to output.
pub fn print_pipeline(
engine_state: &mut EngineState,
stack: &mut Stack,
pipeline: PipelineData,
no_newline: bool,
) -> Result<(), ShellError> {
if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
let pipeline = eval_hook(
engine_state,
stack,
Some(pipeline),
vec![],
&hook,
"display_output",
)?;
pipeline.print_raw(engine_state, no_newline, false)
} else {
// if display_output isn't set, we should still prefer to print with some formatting
pipeline.print_table(engine_state, stack, no_newline, false)
}
}
pub fn eval_source(
engine_state: &mut EngineState,
stack: &mut Stack,
@ -281,36 +310,12 @@ fn evaluate_source(
eval_block::<WithoutDebug>(engine_state, stack, &block, input)
}?;
if let PipelineData::ByteStream(..) = pipeline {
// run the display hook on bytestreams too
run_display_hook(engine_state, stack, pipeline, false)
} else {
run_display_hook(engine_state, stack, pipeline, true)
}?;
let no_newline = matches!(&pipeline, &PipelineData::ByteStream(..));
print_pipeline(engine_state, stack, pipeline, no_newline)?;
Ok(false)
}
fn run_display_hook(
engine_state: &mut EngineState,
stack: &mut Stack,
pipeline: PipelineData,
no_newline: bool,
) -> Result<(), ShellError> {
if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
let pipeline = eval_hook(
engine_state,
stack,
Some(pipeline),
vec![],
&hook,
"display_output",
)?;
pipeline.print(engine_state, stack, no_newline, false)
} else {
pipeline.print(engine_state, stack, no_newline, false)
}
}
#[cfg(test)]
mod test {
use super::*;

View File

@ -194,7 +194,7 @@ impl Command for Watch {
match result {
Ok(val) => {
val.print(engine_state, stack, false, false)?;
val.print_table(engine_state, stack, false, false)?;
}
Err(err) => {
let working_set = StateWorkingSet::new(engine_state);

View File

@ -203,7 +203,7 @@ impl PipelineData {
) -> Result<Self, ShellError> {
match stack.pipe_stdout().unwrap_or(&OutDest::Inherit) {
OutDest::Print => {
self.print(engine_state, stack, false, false)?;
self.print_table(engine_state, stack, false, false)?;
Ok(Self::Empty)
}
OutDest::Pipe | OutDest::PipeSeparate => Ok(self),
@ -534,11 +534,14 @@ impl PipelineData {
}
}
/// Consume and print self data immediately.
/// Consume and print self data immediately, formatted using table command.
///
/// This does not respect the display_output hook. If a value is being printed out by a command,
/// this function should be used. Otherwise, `nu_cli::util::print_pipeline` should be preferred.
///
/// `no_newline` controls if we need to attach newline character to output.
/// `to_stderr` controls if data is output to stderr, when the value is false, the data is output to stdout.
pub fn print(
pub fn print_table(
self,
engine_state: &EngineState,
stack: &mut Stack,