holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add and use new Signals struct (#13314)

Browse Source 
# Description
This PR introduces a new `Signals` struct to replace our adhoc passing
around of `ctrlc: Option<Arc<AtomicBool>>`. Doing so has a few benefits:
- We can better enforce when/where resetting or triggering an interrupt
is allowed.
- Consolidates `nu_utils::ctrl_c::was_pressed` and other ad-hoc
re-implementations into a single place: `Signals::check`.
- This allows us to add other types of signals later if we want. E.g.,
exiting or suspension.
- Similarly, we can more easily change the underlying implementation if
we need to in the future.
- Places that used to have a `ctrlc` of `None` now use
`Signals::empty()`, so we can double check these usages for correctness
in the future.
This commit is contained in:
Ian Manske
2024-07-07 22:29:01 +00:00
committed by GitHub
parent c6b6b1b7a8
commit 399a7c8836
246 changed files with 1332 additions and 1234 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
crates/nu-plugin-engine/src/interface/mod.rs
View File

@@ -12,11 +12,11 @@ use nu_plugin_protocol::{
};
use nu_protocol::{
ast::Operator, CustomValue, IntoSpanned, PipelineData, PluginMetadata, PluginSignature,
ShellError, Span, Spanned, Value,
ShellError, Signals, Span, Spanned, Value,
};
use std::{
collections::{btree_map, BTreeMap},
sync::{atomic::AtomicBool, mpsc, Arc, OnceLock},
sync::{mpsc, Arc, OnceLock},
};
use crate::{
@@ -103,8 +103,8 @@ struct PluginCallState {
/// Don't try to send the plugin call response. This is only used for `Dropped` to avoid an
/// error
dont_send_response: bool,
/// Interrupt signal to be used for stream iterators
ctrlc: Option<Arc<AtomicBool>>,
/// Signals to be used for stream iterators
signals: Signals,
/// Channel to receive context on to be used if needed
context_rx: Option<mpsc::Receiver<Context>>,
/// Span associated with the call, if any
@@ -231,14 +231,14 @@ impl PluginInterfaceManager {
}
}
/// Find the ctrlc signal corresponding to the given plugin call id
fn get_ctrlc(&mut self, id: PluginCallId) -> Result<Option<Arc<AtomicBool>>, ShellError> {
/// Find the [`Signals`] struct corresponding to the given plugin call id
fn get_signals(&mut self, id: PluginCallId) -> Result<Signals, ShellError> {
// Make sure we're up to date
self.receive_plugin_call_subscriptions();
// Find the subscription and return the context
self.plugin_call_states
.get(&id)
.map(|state| state.ctrlc.clone())
.map(|state| state.signals.clone())
.ok_or_else(|| ShellError::PluginFailedToDecode {
msg: format!("Unknown plugin call ID: {id}"),
})
@@ -517,14 +517,14 @@ impl InterfaceManager for PluginInterfaceManager {
// Handle reading the pipeline data, if any
let response = response
.map_data(|data| {
let ctrlc = self.get_ctrlc(id)?;
let signals = self.get_signals(id)?;
// Register the stream in the response
if let Some(stream_id) = data.stream_id() {
self.recv_stream_started(id, stream_id);
}
self.read_pipeline_data(data, ctrlc.as_ref())
self.read_pipeline_data(data, &signals)
})
.unwrap_or_else(|err| {
// If there's an error with initializing this stream, change it to a plugin
@@ -544,8 +544,8 @@ impl InterfaceManager for PluginInterfaceManager {
let call = call
// Handle reading the pipeline data, if any
.map_data(|input| {
let ctrlc = self.get_ctrlc(context)?;
self.read_pipeline_data(input, ctrlc.as_ref())
let signals = self.get_signals(context)?;
self.read_pipeline_data(input, &signals)
})
// Do anything extra needed for each engine call setup
.and_then(|mut engine_call| {
@@ -698,7 +698,9 @@ impl PluginInterface {
context: Option<&dyn PluginExecutionContext>,
) -> Result<WritePluginCallResult, ShellError> {
let id = self.state.plugin_call_id_sequence.next()?;
let ctrlc = context.and_then(|c| c.ctrlc().cloned());
let signals = context
.map(|c| c.signals().clone())
.unwrap_or_else(Signals::empty);
let (tx, rx) = mpsc::channel();
let (context_tx, context_rx) = mpsc::channel();
let keep_plugin_custom_values = mpsc::channel();
@@ -746,7 +748,7 @@ impl PluginInterface {
PluginCallState {
sender: Some(tx).filter(|_| !dont_send_response),
dont_send_response,
ctrlc,
signals,
context_rx: Some(context_rx),
span: call.span(),
keep_plugin_custom_values,

30
crates/nu-plugin-engine/src/interface/tests.rs
View File

@@ -18,7 +18,7 @@ use nu_protocol::{
ast::{Math, Operator},
engine::Closure,
ByteStreamType, CustomValue, IntoInterruptiblePipelineData, IntoSpanned, PipelineData,
PluginMetadata, PluginSignature, ShellError, Span, Spanned, Value,
PluginMetadata, PluginSignature, ShellError, Signals, Span, Spanned, Value,
};
use serde::{Deserialize, Serialize};
use std::{
@@ -56,7 +56,7 @@ fn manager_consume_all_exits_after_streams_and_interfaces_are_dropped() -> Resul
id: 0,
span: Span::test_data(),
}),
None,
&Signals::empty(),
)?;
// and an interface...
@@ -112,7 +112,7 @@ fn manager_consume_all_propagates_io_error_to_readers() -> Result<(), ShellError
id: 0,
span: Span::test_data(),
}),
None,
&Signals::empty(),
)?;
manager
@@ -159,7 +159,7 @@ fn manager_consume_all_propagates_message_error_to_readers() -> Result<(), Shell
span: Span::test_data(),
type_: ByteStreamType::Unknown,
}),
None,
&Signals::empty(),
)?;
manager
@@ -190,7 +190,7 @@ fn fake_plugin_call(
PluginCallState {
sender: Some(tx),
dont_send_response: false,
ctrlc: None,
signals: Signals::empty(),
context_rx: None,
span: None,
keep_plugin_custom_values: mpsc::channel(),
@@ -493,7 +493,7 @@ fn manager_handle_engine_call_after_response_received() -> Result<(), ShellError
PluginCallState {
sender: None,
dont_send_response: false,
ctrlc: None,
signals: Signals::empty(),
context_rx: Some(context_rx),
span: None,
keep_plugin_custom_values: mpsc::channel(),
@@ -559,7 +559,7 @@ fn manager_send_plugin_call_response_removes_context_only_if_no_streams_to_read(
PluginCallState {
sender: None,
dont_send_response: false,
ctrlc: None,
signals: Signals::empty(),
context_rx: None,
span: None,
keep_plugin_custom_values: mpsc::channel(),
@@ -595,7 +595,7 @@ fn manager_consume_stream_end_removes_context_only_if_last_stream() -> Result<()
PluginCallState {
sender: None,
dont_send_response: false,
ctrlc: None,
signals: Signals::empty(),
context_rx: None,
span: None,
keep_plugin_custom_values: mpsc::channel(),
@@ -678,7 +678,7 @@ fn manager_prepare_pipeline_data_adds_source_to_list_streams() -> Result<(), She
[Value::test_custom_value(Box::new(
test_plugin_custom_value(),
))]
.into_pipeline_data(Span::test_data(), None),
.into_pipeline_data(Span::test_data(), Signals::empty()),
)?;
let value = data
@@ -852,7 +852,9 @@ fn interface_write_plugin_call_writes_run_with_stream_input() -> Result<(), Shel
positional: vec![],
named: vec![],
},
input: values.clone().into_pipeline_data(Span::test_data(), None),
input: values
.clone()
.into_pipeline_data(Span::test_data(), Signals::empty()),
}),
None,
)?;
@@ -1148,7 +1150,9 @@ fn interface_prepare_pipeline_data_accepts_normal_streams() -> Result<(), ShellE
let values = normal_values(&interface);
let state = CurrentCallState::default();
let data = interface.prepare_pipeline_data(
values.clone().into_pipeline_data(Span::test_data(), None),
values
.clone()
.into_pipeline_data(Span::test_data(), Signals::empty()),
&state,
)?;
@@ -1211,7 +1215,9 @@ fn interface_prepare_pipeline_data_rejects_bad_custom_value_in_a_stream() -> Res
let values = bad_custom_values();
let state = CurrentCallState::default();
let data = interface.prepare_pipeline_data(
values.clone().into_pipeline_data(Span::test_data(), None),
values
.clone()
.into_pipeline_data(Span::test_data(), Signals::empty()),
&state,
)?;