Make hook execution stream instead of collecting (#7440)

Closes #7431. In a nutshell:
- `run_hook_block()` in repl.rs was collecting all input into a `Value`
instead of handling streaming input properly
- this was a problem because now we have a default `display_output` hook
that _everything_ gets piped to
- this PR fixes the problem by tweaking `run_hook_block()` to return a
`PipelineData` instead of a `Value`

After this change, individual pages are rendered as they finish. This is
a little easier to see if I tweak `STREAM_PAGE_SIZE` in table.rs to 10:

![image](https://user-images.githubusercontent.com/26268125/206935370-412b2ee9-9401-4222-bc93-5bd5a9adc13b.png)

## Future work

This does _not_ fix https://github.com/nushell/nushell/issues/7342.
This commit is contained in:
Reilly Wood 2022-12-12 15:23:04 -08:00 committed by GitHub
parent 5b5f1d1b92
commit b7a3e5989d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -709,6 +709,13 @@ pub fn eval_hook(
) -> Result<PipelineData, ShellError> { ) -> Result<PipelineData, ShellError> {
let value_span = value.span()?; let value_span = value.span()?;
// Hooks can optionally be a record in this form:
// {
// condition: {|before, after| ... } # block that evaluates to true/false
// code: # block or a string
// }
// The condition block will be run to check whether the main hook (in `code`) should be run.
// If it returns true (the default if a condition block is not specified), the hook should be run.
let condition_path = PathMember::String { let condition_path = PathMember::String {
val: "condition".to_string(), val: "condition".to_string(),
span: value_span, span: value_span,
@ -748,16 +755,19 @@ pub fn eval_hook(
arguments.clone(), arguments.clone(),
block_span, block_span,
) { ) {
Ok(value) => match value { Ok(pipeline_data) => {
Value::Bool { val, .. } => val, if let PipelineData::Value(Value::Bool { val, .. }, ..) =
other => { pipeline_data
{
val
} else {
return Err(ShellError::UnsupportedConfigValue( return Err(ShellError::UnsupportedConfigValue(
"boolean output".to_string(), "boolean output".to_string(),
format!("{}", other.get_type()), "other PipelineData variant".to_string(),
other.span()?, block_span,
)); ));
} }
}, }
Err(err) => { Err(err) => {
return Err(err); return Err(err);
} }
@ -880,34 +890,28 @@ pub fn eval_hook(
span: block_span, span: block_span,
.. ..
} => { } => {
output = PipelineData::Value( output = run_hook_block(
run_hook_block(
engine_state, engine_state,
stack, stack,
*block_id, *block_id,
input, input,
arguments, arguments,
*block_span, *block_span,
)?, )?;
None,
);
} }
Value::Closure { Value::Closure {
val: block_id, val: block_id,
span: block_span, span: block_span,
.. ..
} => { } => {
output = PipelineData::Value( output = run_hook_block(
run_hook_block(
engine_state, engine_state,
stack, stack,
*block_id, *block_id,
input, input,
arguments, arguments,
*block_span, *block_span,
)?, )?;
None,
);
} }
other => { other => {
return Err(ShellError::UnsupportedConfigValue( return Err(ShellError::UnsupportedConfigValue(
@ -931,7 +935,7 @@ pub fn run_hook_block(
optional_input: Option<PipelineData>, optional_input: Option<PipelineData>,
arguments: Vec<(String, Value)>, arguments: Vec<(String, Value)>,
span: Span, span: Span,
) -> Result<Value, ShellError> { ) -> Result<PipelineData, ShellError> {
let block = engine_state.get_block(block_id); let block = engine_state.get_block(block_id);
let input = optional_input.unwrap_or_else(PipelineData::empty); let input = optional_input.unwrap_or_else(PipelineData::empty);
@ -955,9 +959,11 @@ pub fn run_hook_block(
match eval_block_with_early_return(engine_state, &mut callee_stack, block, input, false, false) match eval_block_with_early_return(engine_state, &mut callee_stack, block, input, false, false)
{ {
Ok(pipeline_data) => match pipeline_data.into_value(span) { Ok(pipeline_data) => {
Value::Error { error } => Err(error), if let PipelineData::Value(Value::Error { error }, _) = pipeline_data {
val => { return Err(error);
}
// If all went fine, preserve the environment of the called block // If all went fine, preserve the environment of the called block
let caller_env_vars = stack.get_env_var_names(engine_state); let caller_env_vars = stack.get_env_var_names(engine_state);
@ -973,10 +979,8 @@ pub fn run_hook_block(
for (var, value) in callee_stack.get_stack_env_vars() { for (var, value) in callee_stack.get_stack_env_vars() {
stack.add_env_var(var, value); stack.add_env_var(var, value);
} }
Ok(pipeline_data)
Ok(val)
} }
},
Err(err) => Err(err), Err(err) => Err(err),
} }
} }