D10009: rust: Add a `ConfigValueParseError` variant to common errors
SimonSapin
phabricator at mercurial-scm.org
Wed Feb 17 12:59:32 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
Configuration files are parsed into sections of key/value pairs when
they are read, but at that point values are still arbitrary bytes.
Only when a value is accessed by various parts of the code do we know
its expected type and syntax, so values are parsed at that point.
Letâs make a new error type for this latter kind of parsing error,
and add a variant to the common `HgError` so that most code can propagate it
without much boilerplate.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D10009
AFFECTED FILES
rust/hg-core/src/config.rs
rust/hg-core/src/config/config.rs
rust/hg-core/src/errors.rs
CHANGE DETAILS
diff --git a/rust/hg-core/src/errors.rs b/rust/hg-core/src/errors.rs
--- a/rust/hg-core/src/errors.rs
+++ b/rust/hg-core/src/errors.rs
@@ -1,7 +1,8 @@
+use crate::config::ConfigValueParseError;
use std::fmt;
/// Common error cases that can happen in many different APIs
-#[derive(Debug)]
+#[derive(Debug, derive_more::From)]
pub enum HgError {
IoError {
error: std::io::Error,
@@ -29,6 +30,14 @@
/// The given string is a short explanation for users, not intended to be
/// machine-readable.
Abort(String),
+
+ /// A configuration value is not in the expected syntax.
+ ///
+ /// These errors can happen in many places in the code because values are
+ /// parsed lazily as the file-level parser does not know the expected type
+ /// and syntax of each value.
+ #[from]
+ ConfigValueParseError(ConfigValueParseError),
}
/// Details about where an I/O error happened
@@ -63,6 +72,7 @@
impl fmt::Display for HgError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
+ HgError::Abort(explanation) => write!(f, "{}", explanation),
HgError::IoError { error, context } => {
write!(f, "{}: {}", error, context)
}
@@ -72,7 +82,25 @@
HgError::UnsupportedFeature(explanation) => {
write!(f, "unsupported feature: {}", explanation)
}
- HgError::Abort(explanation) => explanation.fmt(f),
+ HgError::ConfigValueParseError(ConfigValueParseError {
+ origin: _,
+ line: _,
+ section,
+ item,
+ value,
+ expected_type,
+ }) => {
+ // TODO: add origin and line number information, here and in
+ // corresponding python code
+ write!(
+ f,
+ "config error: {}.{} is not a {} ('{}')",
+ String::from_utf8_lossy(section),
+ String::from_utf8_lossy(item),
+ expected_type,
+ String::from_utf8_lossy(value)
+ )
+ }
}
}
}
diff --git a/rust/hg-core/src/config/config.rs b/rust/hg-core/src/config/config.rs
--- a/rust/hg-core/src/config/config.rs
+++ b/rust/hg-core/src/config/config.rs
@@ -9,7 +9,7 @@
use super::layer;
use crate::config::layer::{
- ConfigError, ConfigLayer, ConfigParseError, ConfigValue,
+ ConfigError, ConfigLayer, ConfigOrigin, ConfigValue,
};
use crate::utils::files::get_bytes_from_os_str;
use format_bytes::{write_bytes, DisplayBytes};
@@ -54,6 +54,16 @@
Parsed(layer::ConfigLayer),
}
+#[derive(Debug)]
+pub struct ConfigValueParseError {
+ pub origin: ConfigOrigin,
+ pub line: Option<usize>,
+ pub section: Vec<u8>,
+ pub item: Vec<u8>,
+ pub value: Vec<u8>,
+ pub expected_type: &'static str,
+}
+
pub fn parse_bool(v: &[u8]) -> Option<bool> {
match v.to_ascii_lowercase().as_slice() {
b"1" | b"yes" | b"true" | b"on" | b"always" => Some(true),
@@ -262,15 +272,19 @@
&'config self,
section: &[u8],
item: &[u8],
+ expected_type: &'static str,
parse: impl Fn(&'config [u8]) -> Option<T>,
- ) -> Result<Option<T>, ConfigParseError> {
+ ) -> Result<Option<T>, ConfigValueParseError> {
match self.get_inner(§ion, &item) {
Some((layer, v)) => match parse(&v.bytes) {
Some(b) => Ok(Some(b)),
- None => Err(ConfigParseError {
+ None => Err(ConfigValueParseError {
origin: layer.origin.to_owned(),
line: v.line,
- bytes: v.bytes.to_owned(),
+ value: v.bytes.to_owned(),
+ section: section.to_owned(),
+ item: item.to_owned(),
+ expected_type,
}),
},
None => Ok(None),
@@ -283,8 +297,10 @@
&self,
section: &[u8],
item: &[u8],
- ) -> Result<Option<&str>, ConfigParseError> {
- self.get_parse(section, item, |value| str::from_utf8(value).ok())
+ ) -> Result<Option<&str>, ConfigValueParseError> {
+ self.get_parse(section, item, "ASCII or UTF-8 string", |value| {
+ str::from_utf8(value).ok()
+ })
}
/// Returns an `Err` if the first value found is not a valid unsigned
@@ -293,8 +309,8 @@
&self,
section: &[u8],
item: &[u8],
- ) -> Result<Option<u32>, ConfigParseError> {
- self.get_parse(section, item, |value| {
+ ) -> Result<Option<u32>, ConfigValueParseError> {
+ self.get_parse(section, item, "valid integer", |value| {
str::from_utf8(value).ok()?.parse().ok()
})
}
@@ -306,8 +322,8 @@
&self,
section: &[u8],
item: &[u8],
- ) -> Result<Option<u64>, ConfigParseError> {
- self.get_parse(section, item, parse_byte_size)
+ ) -> Result<Option<u64>, ConfigValueParseError> {
+ self.get_parse(section, item, "byte quantity", parse_byte_size)
}
/// Returns an `Err` if the first value found is not a valid boolean.
@@ -317,8 +333,8 @@
&self,
section: &[u8],
item: &[u8],
- ) -> Result<Option<bool>, ConfigParseError> {
- self.get_parse(section, item, parse_bool)
+ ) -> Result<Option<bool>, ConfigValueParseError> {
+ self.get_parse(section, item, "boolean", parse_bool)
}
/// Returns the corresponding boolean in the config. Returns `Ok(false)`
@@ -327,7 +343,7 @@
&self,
section: &[u8],
item: &[u8],
- ) -> Result<bool, ConfigError> {
+ ) -> Result<bool, ConfigValueParseError> {
Ok(self.get_option(section, item)?.unwrap_or(false))
}
diff --git a/rust/hg-core/src/config.rs b/rust/hg-core/src/config.rs
--- a/rust/hg-core/src/config.rs
+++ b/rust/hg-core/src/config.rs
@@ -11,5 +11,5 @@
mod config;
mod layer;
-pub use config::Config;
+pub use config::{Config, ConfigValueParseError};
pub use layer::{ConfigError, ConfigParseError};
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list