Make Span mandatory for some fields in ShellError (#16471)

Some variants of `ShellError` have `Option<Span>` that really don't need
to. This PR removes the `Option` for `ConfigDirNotFound`
and `DisabledOsSupport`.

Also removes `ShellError::InterruptedByUser`, which had exactly one
usage, in favor of the more widely used `ShellError::Interrupted`

Rel: #13005

## Release notes summary - What our users need to know
N/A
This commit is contained in:
rose
2025-08-20 09:56:06 -04:00
committed by GitHub
parent 4f1300bdf6
commit abeb787115
9 changed files with 17 additions and 26 deletions

View File

@@ -49,7 +49,7 @@ impl Command for History {
}; };
// todo for sqlite history this command should be an alias to `open ~/.config/nushell/history.sqlite3 | get history` // todo for sqlite history this command should be an alias to `open ~/.config/nushell/history.sqlite3 | get history`
let Some(history_path) = history.file_path() else { let Some(history_path) = history.file_path() else {
return Err(ShellError::ConfigDirNotFound { span: Some(head) }); return Err(ShellError::ConfigDirNotFound { span: head });
}; };
if call.has_flag(engine_state, stack, "clear")? { if call.has_flag(engine_state, stack, "clear")? {

View File

@@ -78,7 +78,7 @@ Note that history item IDs are ignored when importing from file."#
return ok; return ok;
}; };
let Some(current_history_path) = history.file_path() else { let Some(current_history_path) = history.file_path() else {
return Err(ShellError::ConfigDirNotFound { span: span.into() }); return Err(ShellError::ConfigDirNotFound { span });
}; };
if let Some(bak_path) = backup(&current_history_path, span)? { if let Some(bak_path) = backup(&current_history_path, span)? {
println!("Backed history to {}", bak_path.display()); println!("Backed history to {}", bak_path.display());
@@ -89,8 +89,9 @@ Note that history item IDs are ignored when importing from file."#
HistoryFileFormat::Sqlite => HistoryFileFormat::Plaintext, HistoryFileFormat::Sqlite => HistoryFileFormat::Plaintext,
HistoryFileFormat::Plaintext => HistoryFileFormat::Sqlite, HistoryFileFormat::Plaintext => HistoryFileFormat::Sqlite,
}; };
let src = new_backend(other_format, None)?; let src = new_backend(other_format, None, call.head)?;
let mut dst = new_backend(history.file_format, Some(current_history_path))?; let mut dst =
new_backend(history.file_format, Some(current_history_path), call.head)?;
let items = src let items = src
.search(SearchQuery::everything( .search(SearchQuery::everything(
reedline::SearchDirection::Forward, reedline::SearchDirection::Forward,
@@ -104,7 +105,8 @@ Note that history item IDs are ignored when importing from file."#
_ => { _ => {
let input = input.into_iter().map(item_from_value); let input = input.into_iter().map(item_from_value);
import( import(
new_backend(history.file_format, Some(current_history_path))?.as_mut(), new_backend(history.file_format, Some(current_history_path), call.head)?
.as_mut(),
input, input,
) )
} }
@@ -117,12 +119,13 @@ Note that history item IDs are ignored when importing from file."#
fn new_backend( fn new_backend(
format: HistoryFileFormat, format: HistoryFileFormat,
path: Option<PathBuf>, path: Option<PathBuf>,
span: Span,
) -> Result<Box<dyn History>, ShellError> { ) -> Result<Box<dyn History>, ShellError> {
let path = match path { let path = match path {
Some(path) => path, Some(path) => path,
None => { None => {
let Some(mut path) = nu_path::nu_config_dir() else { let Some(mut path) = nu_path::nu_config_dir() else {
return Err(ShellError::ConfigDirNotFound { span: None }); return Err(ShellError::ConfigDirNotFound { span });
}; };
path.push(format.default_file_name()); path.push(format.default_file_name());
path.into_std_path_buf() path.into_std_path_buf()

View File

@@ -83,7 +83,7 @@ impl Command for Do {
#[cfg(not(feature = "os"))] #[cfg(not(feature = "os"))]
return Err(ShellError::DisabledOsSupport { return Err(ShellError::DisabledOsSupport {
msg: "Cannot create a thread to receive stdout message.".to_string(), msg: "Cannot create a thread to receive stdout message.".to_string(),
span: Some(span), span,
}); });
#[cfg(feature = "os")] #[cfg(feature = "os")]

View File

@@ -52,7 +52,7 @@ pub(super) fn start_editor(
) -> Result<PipelineData, ShellError> { ) -> Result<PipelineData, ShellError> {
Err(ShellError::DisabledOsSupport { Err(ShellError::DisabledOsSupport {
msg: "Running external commands is not available without OS support.".to_string(), msg: "Running external commands is not available without OS support.".to_string(),
span: Some(call.head), span: call.head,
}) })
} }

View File

@@ -45,7 +45,7 @@ impl Command for ConfigReset {
let no_backup = call.has_flag(engine_state, stack, "without-backup")?; let no_backup = call.has_flag(engine_state, stack, "without-backup")?;
let span = call.head; let span = call.head;
let Some(config_path) = nu_path::nu_config_dir() else { let Some(config_path) = nu_path::nu_config_dir() else {
return Err(ShellError::ConfigDirNotFound { span: None }); return Err(ShellError::ConfigDirNotFound { span: call.head });
}; };
if !only_env { if !only_env {
let mut nu_config = config_path.clone(); let mut nu_config = config_path.clone();

View File

@@ -152,9 +152,7 @@ The `prefix` is not included in the output."
.map_err(|err| IoError::new(err, call.head, None))?; .map_err(|err| IoError::new(err, call.head, None))?;
if b[0] == CTRL_C { if b[0] == CTRL_C {
return Err(ShellError::InterruptedByUser { return Err(ShellError::Interrupted { span: call.head });
span: Some(call.head),
});
} }
buf.push(b[0]); buf.push(b[0]);

View File

@@ -1114,13 +1114,6 @@ pub enum ShellError {
span: Span, span: Span,
}, },
/// Operation interrupted by user
#[error("Operation interrupted by user")]
InterruptedByUser {
#[label("This operation was interrupted")]
span: Option<Span>,
},
/// An attempt to use, as a match guard, an expression that /// An attempt to use, as a match guard, an expression that
/// does not resolve into a boolean /// does not resolve into a boolean
#[error("Match guard not bool")] #[error("Match guard not bool")]
@@ -1338,7 +1331,7 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"#
)] )]
ConfigDirNotFound { ConfigDirNotFound {
#[label = "Could not find config directory"] #[label = "Could not find config directory"]
span: Option<Span>, span: Span,
}, },
/// XDG_CONFIG_HOME was set to an invalid path /// XDG_CONFIG_HOME was set to an invalid path
@@ -1378,7 +1371,7 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"#
DisabledOsSupport { DisabledOsSupport {
msg: String, msg: String,
#[label = "while running this code"] #[label = "while running this code"]
span: Option<Span>, span: Span,
}, },
#[error(transparent)] #[error(transparent)]

View File

@@ -38,10 +38,7 @@ pub(crate) fn create_nu_constant(engine_state: &EngineState, span: Span) -> Valu
let config_path = match nu_path::nu_config_dir() { let config_path = match nu_path::nu_config_dir() {
Some(path) => Ok(canonicalize_path(engine_state, path.as_ref())), Some(path) => Ok(canonicalize_path(engine_state, path.as_ref())),
None => Err(Value::error( None => Err(Value::error(ShellError::ConfigDirNotFound { span }, span)),
ShellError::ConfigDirNotFound { span: Some(span) },
span,
)),
}; };
record.push( record.push(

View File

@@ -381,7 +381,7 @@ impl ByteStream {
pub fn stdin(span: Span) -> Result<Self, ShellError> { pub fn stdin(span: Span) -> Result<Self, ShellError> {
Err(ShellError::DisabledOsSupport { Err(ShellError::DisabledOsSupport {
msg: "Stdin is not supported".to_string(), msg: "Stdin is not supported".to_string(),
span: Some(span), span,
}) })
} }