mirror of
https://github.com/nushell/nushell.git
synced 2025-08-09 11:45:50 +02:00
Document and critically review ShellError
variants - Ep. 3 (#8340)
Continuation of #8229 and #8326 # Description The `ShellError` enum at the moment is kind of messy. Many variants are basic tuple structs where you always have to reference the implementation with its macro invocation to know which field serves which purpose. Furthermore we have both variants that are kind of redundant or either overly broad to be useful for the user to match on or overly specific with few uses. So I set out to start fixing the lacking documentation and naming to make it feasible to critically review the individual usages and fix those. Furthermore we can decide to join or split up variants that don't seem to be fit for purpose. # Call to action **Everyone:** Feel free to add review comments if you spot inconsistent use of `ShellError` variants. # User-Facing Changes (None now, end goal more explicit and consistent error messages) # Tests + Formatting (No additional tests needed so far) # Commits (so far) - Remove `ShellError::FeatureNotEnabled` - Name fields on `SE::ExternalNotSupported` - Name field on `SE::InvalidProbability` - Name fields on `SE::NushellFailed` variants - Remove unused `SE::NushellFailedSpannedHelp` - Name field on `SE::VariableNotFoundAtRuntime` - Name fields on `SE::EnvVarNotFoundAtRuntime` - Name fields on `SE::ModuleNotFoundAtRuntime` - Remove usused `ModuleOrOverlayNotFoundAtRuntime` - Name fields on `SE::OverlayNotFoundAtRuntime` - Name field on `SE::NotFound`
This commit is contained in:
committed by
GitHub
parent
4898750fc1
commit
62575c9a4f
@ -60,11 +60,11 @@ impl Command for Const {
|
||||
|
||||
Ok(PipelineData::empty())
|
||||
} else {
|
||||
Err(ShellError::NushellFailedSpanned(
|
||||
"Missing Constant".to_string(),
|
||||
"constant not added by the parser".to_string(),
|
||||
call.head,
|
||||
))
|
||||
Err(ShellError::NushellFailedSpanned {
|
||||
msg: "Missing Constant".to_string(),
|
||||
label: "constant not added by the parser".to_string(),
|
||||
span: call.head,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -161,11 +161,12 @@ impl Command for Do {
|
||||
let stdout = if let Some(handle) = stdout_handler {
|
||||
match handle.join() {
|
||||
Err(err) => {
|
||||
return Err(ShellError::ExternalCommand(
|
||||
"Fail to receive external commands stdout message".to_string(),
|
||||
format!("{err:?}"),
|
||||
return Err(ShellError::ExternalCommand {
|
||||
label: "Fail to receive external commands stdout message"
|
||||
.to_string(),
|
||||
help: format!("{err:?}"),
|
||||
span,
|
||||
));
|
||||
});
|
||||
}
|
||||
Ok(res) => Some(res),
|
||||
}
|
||||
@ -183,11 +184,11 @@ impl Command for Do {
|
||||
};
|
||||
if let Some(Value::Int { val: code, .. }) = exit_code.last() {
|
||||
if *code != 0 {
|
||||
return Err(ShellError::ExternalCommand(
|
||||
"External command failed".to_string(),
|
||||
stderr_msg,
|
||||
return Err(ShellError::ExternalCommand {
|
||||
label: "External command failed".to_string(),
|
||||
help: stderr_msg,
|
||||
span,
|
||||
));
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -100,9 +100,15 @@ You can also learn more at https://www.nushell.sh/book/"#;
|
||||
result
|
||||
};
|
||||
|
||||
if let Err(ShellError::ModuleNotFoundAtRuntime(_, _)) = result {
|
||||
if let Err(ShellError::ModuleNotFoundAtRuntime {
|
||||
mod_name: _,
|
||||
span: _,
|
||||
}) = result
|
||||
{
|
||||
let rest_spans: Vec<Span> = rest.iter().map(|arg| arg.span).collect();
|
||||
Err(ShellError::NotFound(span(&rest_spans)))
|
||||
Err(ShellError::NotFound {
|
||||
span: span(&rest_spans),
|
||||
})
|
||||
} else {
|
||||
result
|
||||
}
|
||||
@ -144,11 +150,11 @@ pub fn highlight_search_in_table(
|
||||
let (cols, mut vals, record_span) = if let Value::Record { cols, vals, span } = record {
|
||||
(cols, vals, span)
|
||||
} else {
|
||||
return Err(ShellError::NushellFailedSpanned(
|
||||
"Expected record".to_string(),
|
||||
format!("got {}", record.get_type()),
|
||||
record.span()?,
|
||||
));
|
||||
return Err(ShellError::NushellFailedSpanned {
|
||||
msg: "Expected record".to_string(),
|
||||
label: format!("got {}", record.get_type()),
|
||||
span: record.span()?,
|
||||
});
|
||||
};
|
||||
|
||||
let has_match = cols.iter().zip(vals.iter_mut()).fold(
|
||||
|
@ -120,10 +120,10 @@ pub fn help_modules(
|
||||
let module_id = if let Some(id) = engine_state.find_module(name.as_bytes(), &[]) {
|
||||
id
|
||||
} else {
|
||||
return Err(ShellError::ModuleNotFoundAtRuntime(
|
||||
name,
|
||||
span(&rest.iter().map(|r| r.span).collect::<Vec<Span>>()),
|
||||
));
|
||||
return Err(ShellError::ModuleNotFoundAtRuntime {
|
||||
mod_name: name,
|
||||
span: span(&rest.iter().map(|r| r.span).collect::<Vec<Span>>()),
|
||||
});
|
||||
};
|
||||
|
||||
let module = engine_state.get_module(module_id);
|
||||
|
@ -58,7 +58,10 @@ impl Command for HideEnv {
|
||||
name.span,
|
||||
));
|
||||
} else {
|
||||
return Err(ShellError::EnvVarNotFoundAtRuntime(name.item, name.span));
|
||||
return Err(ShellError::EnvVarNotFoundAtRuntime {
|
||||
envvar_name: name.item,
|
||||
span: name.span,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -111,12 +111,12 @@ impl Command for If {
|
||||
Ok(PipelineData::empty())
|
||||
}
|
||||
}
|
||||
x => Err(ShellError::CantConvert(
|
||||
"bool".into(),
|
||||
x.get_type().to_string(),
|
||||
result.span()?,
|
||||
None,
|
||||
)),
|
||||
x => Err(ShellError::CantConvert {
|
||||
to_type: "bool".into(),
|
||||
from_type: x.get_type().to_string(),
|
||||
span: result.span()?,
|
||||
help: None,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -61,10 +61,10 @@ impl Command for OverlayHide {
|
||||
};
|
||||
|
||||
if !stack.is_overlay_active(&overlay_name.item) {
|
||||
return Err(ShellError::OverlayNotFoundAtRuntime(
|
||||
overlay_name.item,
|
||||
overlay_name.span,
|
||||
));
|
||||
return Err(ShellError::OverlayNotFoundAtRuntime {
|
||||
overlay_name: overlay_name.item,
|
||||
span: overlay_name.span,
|
||||
});
|
||||
}
|
||||
|
||||
let keep_env: Option<Vec<Spanned<String>>> =
|
||||
@ -76,7 +76,12 @@ impl Command for OverlayHide {
|
||||
for name in env_var_names_to_keep.into_iter() {
|
||||
match stack.get_env_var(engine_state, &name.item) {
|
||||
Some(val) => env_vars_to_keep.push((name.item, val.clone())),
|
||||
None => return Err(ShellError::EnvVarNotFoundAtRuntime(name.item, name.span)),
|
||||
None => {
|
||||
return Err(ShellError::EnvVarNotFoundAtRuntime {
|
||||
envvar_name: name.item,
|
||||
span: name.span,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -70,18 +70,18 @@ impl Command for OverlayUse {
|
||||
if let Expr::Overlay(module_id) = overlay_expr.expr {
|
||||
module_id
|
||||
} else {
|
||||
return Err(ShellError::NushellFailedSpanned(
|
||||
"Not an overlay".to_string(),
|
||||
"requires an overlay (path or a string)".to_string(),
|
||||
overlay_expr.span,
|
||||
));
|
||||
return Err(ShellError::NushellFailedSpanned {
|
||||
msg: "Not an overlay".to_string(),
|
||||
label: "requires an overlay (path or a string)".to_string(),
|
||||
span: overlay_expr.span,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
return Err(ShellError::NushellFailedSpanned(
|
||||
"Missing positional".to_string(),
|
||||
"missing required overlay".to_string(),
|
||||
call.head,
|
||||
));
|
||||
return Err(ShellError::NushellFailedSpanned {
|
||||
msg: "Missing positional".to_string(),
|
||||
label: "missing required overlay".to_string(),
|
||||
span: call.head,
|
||||
});
|
||||
};
|
||||
|
||||
let overlay_name = if let Some(name) = call.opt(engine_state, caller_stack, 1)? {
|
||||
@ -98,10 +98,10 @@ impl Command for OverlayUse {
|
||||
return Err(ShellError::NonUtf8(name_arg.span));
|
||||
}
|
||||
} else {
|
||||
return Err(ShellError::OverlayNotFoundAtRuntime(
|
||||
name_arg.item,
|
||||
name_arg.span,
|
||||
));
|
||||
return Err(ShellError::OverlayNotFoundAtRuntime {
|
||||
overlay_name: name_arg.item,
|
||||
span: name_arg.span,
|
||||
});
|
||||
};
|
||||
|
||||
if let Some(module_id) = maybe_origin_module_id {
|
||||
|
@ -89,12 +89,12 @@ impl Command for While {
|
||||
}
|
||||
}
|
||||
x => {
|
||||
return Err(ShellError::CantConvert(
|
||||
"bool".into(),
|
||||
x.get_type().to_string(),
|
||||
result.span()?,
|
||||
None,
|
||||
))
|
||||
return Err(ShellError::CantConvert {
|
||||
to_type: "bool".into(),
|
||||
from_type: x.get_type().to_string(),
|
||||
span: result.span()?,
|
||||
help: None,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user