diff --git a/crates/nu-command/src/experimental/job_kill.rs b/crates/nu-command/src/experimental/job_kill.rs index 316b27a0d7..4e71e678fe 100644 --- a/crates/nu-command/src/experimental/job_kill.rs +++ b/crates/nu-command/src/experimental/job_kill.rs @@ -34,21 +34,13 @@ impl Command for JobKill { ) -> Result { let head = call.head; - let id_arg: Spanned = call.req(engine_state, stack, 0)?; - - if id_arg.item < 0 { - return Err(ShellError::NeedsPositiveValue { span: id_arg.span }); - } - - let id: JobId = JobId::new(id_arg.item as usize); + let id_arg: Spanned = call.req(engine_state, stack, 0)?; + let id = JobId::new(id_arg.item); let mut jobs = engine_state.jobs.lock().expect("jobs lock is poisoned!"); if jobs.lookup(id).is_none() { - return Err(ShellError::JobNotFound { - id: id.get(), - span: head, - }); + return Err(JobError::NotFound { span: head, id }.into()); } jobs.kill_and_remove(id).map_err(|err| { diff --git a/crates/nu-command/src/experimental/job_recv.rs b/crates/nu-command/src/experimental/job_recv.rs index 81861c2d96..c98d7a1783 100644 --- a/crates/nu-command/src/experimental/job_recv.rs +++ b/crates/nu-command/src/experimental/job_recv.rs @@ -145,7 +145,7 @@ fn recv_instantly( ) -> Result { match mailbox.try_recv(tag) { Ok(value) => Ok(value), - Err(TryRecvError::Empty) => Err(ShellError::RecvTimeout { span }), + Err(TryRecvError::Empty) => Err(JobError::RecvTimeout { span }.into()), Err(TryRecvError::Disconnected) => Err(ShellError::Interrupted { span }), } } @@ -175,7 +175,7 @@ fn recv_with_time_limit( } if time_until_deadline.is_zero() { - return Err(ShellError::RecvTimeout { span }); + return Err(JobError::RecvTimeout { span }.into()); } } } diff --git a/crates/nu-command/src/experimental/job_send.rs b/crates/nu-command/src/experimental/job_send.rs index 3df67add77..868c0ed42c 100644 --- a/crates/nu-command/src/experimental/job_send.rs +++ b/crates/nu-command/src/experimental/job_send.rs @@ -52,14 +52,10 @@ This command never blocks. ) -> Result { let head = call.head; - let id_arg: Spanned = call.req(engine_state, stack, 0)?; + let id_arg: Spanned = call.req(engine_state, stack, 0)?; let tag_arg: Option> = call.get_flag(engine_state, stack, "tag")?; - let id = id_arg.item; - - if id < 0 { - return Err(ShellError::NeedsPositiveValue { span: id_arg.span }); - } + let id = JobId::new(id_arg.item); if let Some(tag) = tag_arg { if tag.item < 0 { @@ -69,7 +65,7 @@ This command never blocks. let tag = tag_arg.map(|it| it.item as FilterTag); - if id == 0 { + if id == JobId::ZERO { engine_state .root_job_sender .send((tag, input)) @@ -77,7 +73,7 @@ This command never blocks. } else { let jobs = engine_state.jobs.lock().expect("failed to acquire lock"); - if let Some(job) = jobs.lookup(JobId::new(id as usize)) { + if let Some(job) = jobs.lookup(id) { match job { nu_protocol::engine::Job::Thread(thread_job) => { // it is ok to send this value while holding the lock, because @@ -85,17 +81,19 @@ This command never blocks. let _ = thread_job.sender.send((tag, input)); } nu_protocol::engine::Job::Frozen(_) => { - return Err(ShellError::JobIsFrozen { - id: id as usize, + return Err(JobError::AlreadyFrozen { span: id_arg.span, - }); + id, + } + .into()); } } } else { - return Err(ShellError::JobNotFound { - id: id as usize, + return Err(JobError::NotFound { span: id_arg.span, - }); + id, + } + .into()); } } diff --git a/crates/nu-command/src/experimental/job_tag.rs b/crates/nu-command/src/experimental/job_tag.rs index 651e687736..81cf9c144b 100644 --- a/crates/nu-command/src/experimental/job_tag.rs +++ b/crates/nu-command/src/experimental/job_tag.rs @@ -38,26 +38,15 @@ impl Command for JobTag { ) -> Result { let head = call.head; - let id_arg: Spanned = call.req(engine_state, stack, 0)?; - - if id_arg.item < 0 { - return Err(ShellError::NeedsPositiveValue { span: id_arg.span }); - } - - let id: JobId = JobId::new(id_arg.item as usize); + let id_arg: Spanned = call.req(engine_state, stack, 0)?; + let id = JobId::new(id_arg.item); let tag: Option = call.req(engine_state, stack, 1)?; let mut jobs = engine_state.jobs.lock().expect("jobs lock is poisoned!"); match jobs.lookup_mut(id) { - None => { - return Err(ShellError::JobNotFound { - id: id.get(), - span: head, - }); - } - + None => return Err(JobError::NotFound { span: head, id }.into()), Some(job) => job.assign_tag(tag), } diff --git a/crates/nu-command/src/experimental/job_unfreeze.rs b/crates/nu-command/src/experimental/job_unfreeze.rs index 5b34652ec8..d7538e1779 100644 --- a/crates/nu-command/src/experimental/job_unfreeze.rs +++ b/crates/nu-command/src/experimental/job_unfreeze.rs @@ -39,33 +39,18 @@ impl Command for JobUnfreeze { ) -> Result { let head = call.head; - let option_id: Option> = call.opt(engine_state, stack, 0)?; - let mut jobs = engine_state.jobs.lock().expect("jobs lock is poisoned!"); - if let Some(id_arg) = option_id { - if id_arg.item < 0 { - return Err(ShellError::NeedsPositiveValue { span: id_arg.span }); - } - } - - let id = option_id - .map(|it| JobId::new(it.item as usize)) + let id: Option = call.opt(engine_state, stack, 0)?; + let id = id + .map(JobId::new) .or_else(|| jobs.most_recent_frozen_job_id()) - .ok_or_else(|| ShellError::NoFrozenJob { span: head })?; + .ok_or(JobError::NoneToUnfreeze { span: head })?; let job = match jobs.lookup(id) { - None => { - return Err(ShellError::JobNotFound { - id: id.get(), - span: head, - }); - } + None => return Err(JobError::NotFound { span: head, id }.into()), Some(Job::Thread(ThreadJob { .. })) => { - return Err(ShellError::JobNotFrozen { - id: id.get(), - span: head, - }); + return Err(JobError::CannotUnfreeze { span: head, id }.into()); } Some(Job::Frozen(FrozenJob { .. })) => jobs .remove_job(id) @@ -107,11 +92,7 @@ fn unfreeze_job( span: Span, ) -> Result<(), ShellError> { match job { - Job::Thread(ThreadJob { .. }) => Err(ShellError::JobNotFrozen { - id: old_id.get(), - span, - }), - + Job::Thread(ThreadJob { .. }) => Err(JobError::CannotUnfreeze { span, id: old_id }.into()), Job::Frozen(FrozenJob { unfreeze: handle, tag, diff --git a/crates/nu-engine/src/command_prelude.rs b/crates/nu-engine/src/command_prelude.rs index c6073add4b..702a6a4b51 100644 --- a/crates/nu-engine/src/command_prelude.rs +++ b/crates/nu-engine/src/command_prelude.rs @@ -6,5 +6,5 @@ pub use nu_protocol::{ ast::CellPath, engine::{Call, Command, EngineState, Stack, StateWorkingSet}, record, - shell_error::io::*, + shell_error::{io::*, job::*}, }; diff --git a/crates/nu-protocol/src/errors/shell_error/io.rs b/crates/nu-protocol/src/errors/shell_error/io.rs index 0722cd9abe..a60a7fae0b 100644 --- a/crates/nu-protocol/src/errors/shell_error/io.rs +++ b/crates/nu-protocol/src/errors/shell_error/io.rs @@ -8,7 +8,7 @@ use thiserror::Error; use crate::Span; -use super::{ShellError, location::Location}; +use super::location::Location; /// Represents an I/O error in the [`ShellError::Io`] variant. /// @@ -494,12 +494,6 @@ impl Diagnostic for IoError { } } -impl From for ShellError { - fn from(value: IoError) -> Self { - ShellError::Io(value) - } -} - impl From for std::io::Error { fn from(value: IoError) -> Self { Self::new(value.kind.into(), value) diff --git a/crates/nu-protocol/src/errors/shell_error/job.rs b/crates/nu-protocol/src/errors/shell_error/job.rs new file mode 100644 index 0000000000..41e4306c7b --- /dev/null +++ b/crates/nu-protocol/src/errors/shell_error/job.rs @@ -0,0 +1,45 @@ +use miette::Diagnostic; +use thiserror::Error; + +use crate::{JobId, Span}; + +/// Errors when working working with jobs. +#[derive(Debug, Clone, Copy, PartialEq, Error, Diagnostic)] +pub enum JobError { + #[error("Job {id} not found")] + #[diagnostic( + code(nu::shell::job::not_found), + help( + "The operation could not be completed, there is no job currently running with this id" + ) + )] + NotFound { span: Span, id: JobId }, + + #[error("No frozen job to unfreeze")] + #[diagnostic( + code(nu::shell::job::none_to_unfreeze), + help("There is currently no frozen job to unfreeze") + )] + NoneToUnfreeze { span: Span }, + + #[error("Job {id} is not frozen")] + #[diagnostic( + code(nu::shell::job::cannot_unfreeze), + help("You tried to unfreeze a job which is not frozen") + )] + CannotUnfreeze { span: Span, id: JobId }, + + #[error("The job {id} is frozen")] + #[diagnostic( + code(nu::shell::job::already_frozen), + help("This operation cannot be performed because the job is frozen") + )] + AlreadyFrozen { span: Span, id: JobId }, + + #[error("No message was received in the requested time interval")] + #[diagnostic( + code(nu::shell::job::recv_timeout), + help("No message arrived within the specified time limit") + )] + RecvTimeout { span: Span }, +} diff --git a/crates/nu-protocol/src/errors/shell_error/mod.rs b/crates/nu-protocol/src/errors/shell_error/mod.rs index 6d1631310c..c53958eeee 100644 --- a/crates/nu-protocol/src/errors/shell_error/mod.rs +++ b/crates/nu-protocol/src/errors/shell_error/mod.rs @@ -3,6 +3,7 @@ use crate::{ ConfigError, LabeledError, ParseError, Span, Spanned, Type, Value, ast::Operator, engine::StateWorkingSet, format_shell_error, record, }; +use job::JobError; use miette::Diagnostic; use serde::{Deserialize, Serialize}; use std::num::NonZeroI32; @@ -10,6 +11,7 @@ use thiserror::Error; pub mod bridge; pub mod io; +pub mod job; pub mod location; /// The fundamental error type for the evaluation engine. These cases represent different kinds of errors @@ -917,7 +919,7 @@ pub enum ShellError { /// This is the main I/O error, for further details check the error kind and additional context. #[error(transparent)] #[diagnostic(transparent)] - Io(io::IoError), + Io(#[from] io::IoError), /// A name was not found. Did you mean a different name? /// @@ -1381,60 +1383,9 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"# span: Option, }, - #[error("Job {id} not found")] - #[diagnostic( - code(nu::shell::job_not_found), - help( - "The operation could not be completed, there is no job currently running with this id" - ) - )] - JobNotFound { - id: usize, - #[label = "job not found"] - span: Span, - }, - - #[error("No frozen job to unfreeze")] - #[diagnostic( - code(nu::shell::no_frozen_job), - help("There is currently no frozen job to unfreeze") - )] - NoFrozenJob { - #[label = "no frozen job"] - span: Span, - }, - - #[error("Job {id} is not frozen")] - #[diagnostic( - code(nu::shell::job_not_frozen), - help("You tried to unfreeze a job which is not frozen") - )] - JobNotFrozen { - id: usize, - #[label = "job not frozen"] - span: Span, - }, - - #[error("The job {id} is frozen")] - #[diagnostic( - code(nu::shell::job_is_frozen), - help("This operation cannot be performed because the job is frozen") - )] - JobIsFrozen { - id: usize, - #[label = "This job is frozen"] - span: Span, - }, - - #[error("No message was received in the requested time interval")] - #[diagnostic( - code(nu::shell::recv_timeout), - help("No message arrived within the specified time limit") - )] - RecvTimeout { - #[label = "timeout"] - span: Span, - }, + #[error(transparent)] + #[diagnostic(transparent)] + Job(#[from] JobError), #[error(transparent)] #[diagnostic(transparent)] diff --git a/crates/nu-protocol/src/id.rs b/crates/nu-protocol/src/id.rs index 32f31a94c8..d52c22458e 100644 --- a/crates/nu-protocol/src/id.rs +++ b/crates/nu-protocol/src/id.rs @@ -37,6 +37,10 @@ where } } +impl Id { + pub const ZERO: Self = Self::new(0); +} + impl Debug for Id where V: Display, @@ -115,6 +119,12 @@ pub type JobId = Id; /// Note: `%0` is allocated with the block input at the beginning of a compiled block. pub type RegId = Id; +impl Display for JobId { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.inner) + } +} + impl Display for RegId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "%{}", self.get())