diff --git a/crates/nu-protocol/src/config/error.rs b/crates/nu-protocol/src/config/error.rs index c8fbd3f85a..3a0c23943f 100644 --- a/crates/nu-protocol/src/config/error.rs +++ b/crates/nu-protocol/src/config/error.rs @@ -1,10 +1,12 @@ use super::ConfigPath; -use crate::{Config, ConfigError, ShellError, Span, Type, Value}; +use crate::{Config, ConfigError, ConfigWarning, ShellError, ShellWarning, Span, Type, Value}; #[derive(Debug)] +#[must_use] pub(super) struct ConfigErrors<'a> { config: &'a Config, errors: Vec, + warnings: Vec, } impl<'a> ConfigErrors<'a> { @@ -12,17 +14,26 @@ impl<'a> ConfigErrors<'a> { Self { config, errors: Vec::new(), + warnings: Vec::new(), } } - pub fn is_empty(&self) -> bool { - self.errors.is_empty() + pub fn has_errors(&self) -> bool { + !self.errors.is_empty() + } + + pub fn has_warnings(&self) -> bool { + !self.warnings.is_empty() } pub fn error(&mut self, error: ConfigError) { self.errors.push(error); } + pub fn warn(&mut self, warning: ConfigWarning) { + self.warnings.push(warning); + } + pub fn type_mismatch(&mut self, path: &ConfigPath, expected: Type, actual: &Value) { self.error(ConfigError::TypeMismatch { path: path.to_string(), @@ -75,13 +86,15 @@ impl<'a> ConfigErrors<'a> { }); } - pub fn into_shell_error(self) -> Option { - if self.is_empty() { - None - } else { - Some(ShellError::InvalidConfig { + pub fn check(self) -> Result, ShellError> { + match (self.has_errors(), self.has_warnings()) { + (true, _) => Err(ShellError::InvalidConfig { errors: self.errors, - }) + }), + (false, true) => Ok(Some(ShellWarning::InvalidConfig { + warnings: self.warnings, + })), + (false, false) => Ok(None), } } } diff --git a/crates/nu-protocol/src/config/history.rs b/crates/nu-protocol/src/config/history.rs index 8f7af7c4a5..87df2f3d56 100644 --- a/crates/nu-protocol/src/config/history.rs +++ b/crates/nu-protocol/src/config/history.rs @@ -1,5 +1,5 @@ use super::{config_update_string_enum, prelude::*}; -use crate as nu_protocol; +use crate::{self as nu_protocol, ConfigWarning}; #[derive(Clone, Copy, Debug, IntoValue, PartialEq, Eq, Serialize, Deserialize)] pub enum HistoryFileFormat { @@ -77,15 +77,35 @@ impl UpdateFromValue for HistoryConfig { return; }; + // might not be correct if file format was changed away from sqlite rather than isolation, + // but this is an edge case and the span of the relevant value here should be close enough + let mut isolation_span = value.span(); + for (col, val) in record.iter() { let path = &mut path.push(col); match col.as_str() { - "isolation" => self.isolation.update(val, path, errors), + "isolation" => { + isolation_span = val.span(); + self.isolation.update(val, path, errors) + } "sync_on_enter" => self.sync_on_enter.update(val, path, errors), "max_size" => self.max_size.update(val, path, errors), "file_format" => self.file_format.update(val, path, errors), _ => errors.unknown_option(path, val), } } + + // Listing all formats separately in case additional ones are added + match (self.isolation, self.file_format) { + (true, HistoryFileFormat::Plaintext) => { + errors.warn(ConfigWarning::IncompatibleOptions { + label: "history isolation only compatible with SQLite format", + span: isolation_span, + help: r#"disable history isolation, or set $env.config.history.file_format = "sqlite""#, + }); + } + (true, HistoryFileFormat::Sqlite) => (), + (false, _) => (), + } } } diff --git a/crates/nu-protocol/src/config/mod.rs b/crates/nu-protocol/src/config/mod.rs index 4f267d2597..01ad9f0fff 100644 --- a/crates/nu-protocol/src/config/mod.rs +++ b/crates/nu-protocol/src/config/mod.rs @@ -1,7 +1,7 @@ //! Module containing the internal representation of user configuration -use crate as nu_protocol; use crate::FromValue; +use crate::{self as nu_protocol}; use helper::*; use prelude::*; use std::collections::HashMap; @@ -219,7 +219,11 @@ impl UpdateFromValue for Config { } impl Config { - pub fn update_from_value(&mut self, old: &Config, value: &Value) -> Option { + pub fn update_from_value( + &mut self, + old: &Config, + value: &Value, + ) -> Result, ShellError> { // Current behaviour is that config errors are displayed, but do not prevent the rest // of the config from being updated (fields with errors are skipped/not updated). // Errors are simply collected one-by-one and wrapped into a ShellError variant at the end. @@ -228,6 +232,6 @@ impl Config { self.update(value, &mut path, &mut errors); - errors.into_shell_error() + errors.check() } } diff --git a/crates/nu-protocol/src/config/plugin_gc.rs b/crates/nu-protocol/src/config/plugin_gc.rs index 52d170cd36..7e1d1476b9 100644 --- a/crates/nu-protocol/src/config/plugin_gc.rs +++ b/crates/nu-protocol/src/config/plugin_gc.rs @@ -147,7 +147,7 @@ mod tests { let mut errors = ConfigErrors::new(&config); let mut result = PluginGcConfigs::default(); result.update(&input, &mut ConfigPath::new(), &mut errors); - assert!(errors.is_empty(), "errors: {errors:#?}"); + assert!(!errors.has_errors(), "errors: {errors:#?}"); assert_eq!(expected, result); } diff --git a/crates/nu-protocol/src/config/prelude.rs b/crates/nu-protocol/src/config/prelude.rs index 10adca616a..a7a2e2f3a3 100644 --- a/crates/nu-protocol/src/config/prelude.rs +++ b/crates/nu-protocol/src/config/prelude.rs @@ -1,4 +1,4 @@ pub(super) use super::{ConfigPath, UpdateFromValue, error::ConfigErrors}; -pub use crate::{IntoValue, ShellError, Span, Type, Value, record}; +pub use crate::{IntoValue, ShellError, ShellWarning, Span, Type, Value, record}; pub use serde::{Deserialize, Serialize}; pub use std::str::FromStr; diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index be46327bef..7df005ecd0 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -4,6 +4,7 @@ use crate::{ ArgumentStack, DEFAULT_OVERLAY_NAME, EngineState, ErrorHandlerStack, Redirection, StackCallArgGuard, StackCollectValueGuard, StackIoGuard, StackOutDest, }, + report_shell_warning, }; use nu_utils::IgnoreCaseExt; use std::{ @@ -212,18 +213,17 @@ impl Stack { if let Some(value) = self.get_env_var(engine_state, "config") { let old = self.get_config(engine_state); let mut config = (*old).clone(); - let error = config.update_from_value(&old, value); + let result = config.update_from_value(&old, value); // The config value is modified by the update, so we should add it again self.add_env_var("config".into(), config.clone().into_value(value.span())); self.config = Some(config.into()); - match error { - None => Ok(()), - Some(err) => Err(err), + if let Some(warning) = result? { + report_shell_warning(engine_state, &warning); } } else { self.config = None; - Ok(()) } + Ok(()) } pub fn add_var(&mut self, var_id: VarId, value: Value) { diff --git a/crates/nu-protocol/src/errors/config_error.rs b/crates/nu-protocol/src/errors/config.rs similarity index 66% rename from crates/nu-protocol/src/errors/config_error.rs rename to crates/nu-protocol/src/errors/config.rs index ab1cedfb22..20cddb94dc 100644 --- a/crates/nu-protocol/src/errors/config_error.rs +++ b/crates/nu-protocol/src/errors/config.rs @@ -1,3 +1,5 @@ +use std::hash::Hash; + use crate::{ShellError, Span, Type}; use miette::Diagnostic; use thiserror::Error; @@ -54,3 +56,29 @@ pub enum ConfigError { #[diagnostic(transparent)] ShellError(#[from] ShellError), } + +/// Warnings which don't prevent config from being loaded, but we should inform the user about +#[derive(Clone, Debug, PartialEq, Error, Diagnostic)] +#[diagnostic(severity(Warning))] +pub enum ConfigWarning { + #[error("Incompatible options")] + #[diagnostic(code(nu::shell::incompatible_options), help("{help}"))] + IncompatibleOptions { + label: &'static str, + #[label = "{label}"] + span: Span, + help: &'static str, + }, +} + +// To keep track of reported warnings +impl Hash for ConfigWarning { + fn hash(&self, state: &mut H) { + match self { + ConfigWarning::IncompatibleOptions { label, help, .. } => { + label.hash(state); + help.hash(state); + } + } + } +} diff --git a/crates/nu-protocol/src/errors/mod.rs b/crates/nu-protocol/src/errors/mod.rs index 071d0bb211..d04c3db70f 100644 --- a/crates/nu-protocol/src/errors/mod.rs +++ b/crates/nu-protocol/src/errors/mod.rs @@ -1,6 +1,6 @@ mod chained_error; mod compile_error; -mod config_error; +mod config; mod labeled_error; mod parse_error; mod parse_warning; @@ -9,7 +9,7 @@ pub mod shell_error; pub mod shell_warning; pub use compile_error::CompileError; -pub use config_error::ConfigError; +pub use config::{ConfigError, ConfigWarning}; pub use labeled_error::{ErrorLabel, LabeledError}; pub use parse_error::{DidYouMean, ParseError}; pub use parse_warning::ParseWarning; diff --git a/crates/nu-protocol/src/errors/parse_warning.rs b/crates/nu-protocol/src/errors/parse_warning.rs index 24deb41b66..21980e6158 100644 --- a/crates/nu-protocol/src/errors/parse_warning.rs +++ b/crates/nu-protocol/src/errors/parse_warning.rs @@ -1,14 +1,16 @@ use crate::Span; use miette::Diagnostic; -use serde::{Deserialize, Serialize}; use std::hash::Hash; use thiserror::Error; use crate::{ReportMode, Reportable}; -#[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)] +#[derive(Clone, Debug, Error, Diagnostic)] #[diagnostic(severity(Warning))] pub enum ParseWarning { + /// A parse-time deprectaion. Indicates that something will be removed in a future release. + /// + /// Use [`ShellWarning::Deprecated`] if this is a deprecation which is only detectable at run-time. #[error("{dep_type} deprecated.")] #[diagnostic(code(nu::parser::deprecated))] Deprecated { diff --git a/crates/nu-protocol/src/errors/shell_warning.rs b/crates/nu-protocol/src/errors/shell_warning.rs index 50abeaf21f..3a490e4f54 100644 --- a/crates/nu-protocol/src/errors/shell_warning.rs +++ b/crates/nu-protocol/src/errors/shell_warning.rs @@ -1,14 +1,16 @@ use crate::Span; use miette::Diagnostic; -use serde::{Deserialize, Serialize}; use std::hash::Hash; use thiserror::Error; -use crate::{ReportMode, Reportable}; +use crate::{ConfigWarning, ReportMode, Reportable}; -#[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)] +#[derive(Clone, Debug, Error, Diagnostic)] #[diagnostic(severity(Warning))] pub enum ShellWarning { + /// A parse-time deprectaion. Indicates that something will be removed in a future release. + /// + /// Use [`ParseWarning::Deprecated`] if this is a deprecation which is detectable at parse-time. #[error("{dep_type} deprecated.")] #[diagnostic(code(nu::shell::deprecated))] Deprecated { @@ -20,20 +22,20 @@ pub enum ShellWarning { help: Option, report_mode: ReportMode, }, -} - -impl ShellWarning { - pub fn span(&self) -> Span { - match self { - ShellWarning::Deprecated { span, .. } => *span, - } - } + /// Warnings reported while updating the config + #[error("Encountered {} warnings(s) when updating config", warnings.len())] + #[diagnostic(code(nu::shell::invalid_config))] + InvalidConfig { + #[related] + warnings: Vec, + }, } impl Reportable for ShellWarning { fn report_mode(&self) -> ReportMode { match self { ShellWarning::Deprecated { report_mode, .. } => *report_mode, + ShellWarning::InvalidConfig { .. } => ReportMode::FirstUse, } } } @@ -48,6 +50,8 @@ impl Hash for ShellWarning { dep_type.hash(state); label.hash(state); } + // We always report config warnings, so no hash necessary + ShellWarning::InvalidConfig { .. } => (), } } } diff --git a/tests/repl/test_config.rs b/tests/repl/test_config.rs index 8e69d29439..1218e62c85 100644 --- a/tests/repl/test_config.rs +++ b/tests/repl/test_config.rs @@ -168,3 +168,11 @@ fn mutate_nu_config_plugin_gc_plugins() -> TestResult { "0sec", ) } + +#[test] +fn mutate_nu_config_history_warning() -> TestResult { + fail_test( + r#"$env.config.history.file_format = "plaintext"; $env.config.history.isolation = true"#, + "history isolation only compatible with SQLite format", + ) +}