Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logging feature and expose in icu crate #3648

Merged
merged 1 commit into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion components/icu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ icu_compactdecimal = { version = "0.2.0", path = "../../experimental/compactdeci
icu_displaynames = { version = "0.10.0", path = "../../experimental/displaynames", default-features = false, optional = true }
icu_relativetime = { version = "0.1.0", path = "../../experimental/relativetime", default-features = false, optional = true }

# For docs links
# For docs links and features
icu_provider = { version = "1.2.0", path = "../../provider/core", default-features = false }

[dev-dependencies]
Expand Down Expand Up @@ -141,6 +141,8 @@ experimental = [
"icu_plurals/experimental",
"icu_relativetime",
]
sync = ["icu_provider/sync"]
logging = ["icu_provider/logging"]

[package.metadata.cargo-all-features]
# Components are tested individually, and there's no logic in this crate
Expand Down
2 changes: 1 addition & 1 deletion ffi/diplomat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ buffer_provider = [
]
provider_fs = ["dep:icu_provider_fs", "buffer_provider"]
provider_test = []
logging = ["icu_provider/log_error_context", "dep:log"]
logging = ["icu_provider/logging", "dep:log"]
# Use the env_logger functionality to log based on environment variables
simple_logger = ["dep:simple_logger"]

Expand Down
4 changes: 3 additions & 1 deletion provider/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ sync = []
macros = ["dep:icu_provider_macros"]

# Enable logging of additional context of data errors
log_error_context = ["dep:log"]
logging = ["dep:log"]
# Legacy name
log_error_context = ["logging"]

# Enable BufferProvider and other deserialization infrastructure
serde = ["dep:serde", "yoke/serde"]
Expand Down
26 changes: 13 additions & 13 deletions provider/core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,15 @@ impl DataError {

/// Logs the data error with the given request, returning an error containing the resource key.
///
/// If the "log_error_context" Cargo feature is enabled, this logs the whole request. Either way,
/// If the "logging" Cargo feature is enabled, this logs the whole request. Either way,
/// it returns an error with the resource key portion of the request as context.
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
pub fn with_req(mut self, key: DataKey, req: DataRequest) -> Self {
if req.metadata.silent {
self.silent = true;
}
// Don't write out a log for MissingDataKey since there is no context to add
#[cfg(feature = "log_error_context")]
#[cfg(feature = "logging")]
if !self.silent && self.kind != DataErrorKind::MissingDataKey {
log::warn!("{} (key: {}, request: {})", self, key, req);
}
Expand All @@ -220,12 +220,12 @@ impl DataError {

/// Logs the data error with the given context, then return self.
///
/// This does not modify the error, but if the "log_error_context" Cargo feature is enabled,
/// This does not modify the error, but if the "logging" Cargo feature is enabled,
/// it will print out the context.
#[cfg(feature = "std")]
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
pub fn with_path_context<P: AsRef<std::path::Path> + ?Sized>(self, path: &P) -> Self {
#[cfg(feature = "log_error_context")]
#[cfg(feature = "logging")]
if !self.silent {
log::warn!("{} (path: {:?})", self, path.as_ref());
}
Expand All @@ -234,12 +234,12 @@ impl DataError {

/// Logs the data error with the given context, then return self.
///
/// This does not modify the error, but if the "log_error_context" Cargo feature is enabled,
/// This does not modify the error, but if the "logging" Cargo feature is enabled,
/// it will print out the context.
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
#[inline]
pub fn with_display_context<D: fmt::Display + ?Sized>(self, context: &D) -> Self {
#[cfg(feature = "log_error_context")]
#[cfg(feature = "logging")]
if !self.silent {
log::warn!("{}: {}", self, context);
}
Expand All @@ -248,12 +248,12 @@ impl DataError {

/// Logs the data error with the given context, then return self.
///
/// This does not modify the error, but if the "log_error_context" Cargo feature is enabled,
/// This does not modify the error, but if the "logging" Cargo feature is enabled,
/// it will print out the context.
#[cfg_attr(not(feature = "log_error_context"), allow(unused_variables))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
#[inline]
pub fn with_debug_context<D: fmt::Debug + ?Sized>(self, context: &D) -> Self {
#[cfg(feature = "log_error_context")]
#[cfg(feature = "logging")]
if !self.silent {
log::warn!("{}: {:?}", self, context);
}
Expand All @@ -277,7 +277,7 @@ impl std::error::Error for DataError {}
#[cfg(feature = "std")]
impl From<std::io::Error> for DataError {
fn from(e: std::io::Error) -> Self {
#[cfg(feature = "log_error_context")]
#[cfg(feature = "logging")]
log::warn!("I/O error: {}", e);
DataErrorKind::Io(e.kind()).into_error()
}
Expand Down
2 changes: 1 addition & 1 deletion provider/datagen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ icu_timezone = { version = "1.2.0", path = "../../components/timezone", default-
icu_codepointtrie_builder = { version = "0.3.4", path = "../../components/collections/codepointtrie_builder", default-features = false }
icu_collections = { version = "1.2.0", path = "../../components/collections", features = ["serde"] }
icu_locid = { version = "1.2.0", path = "../../components/locid", features = ["std", "serde"] }
icu_provider = { version = "1.2.0", path = "../core", features = ["std", "log_error_context", "datagen"]}
icu_provider = { version = "1.2.0", path = "../core", features = ["std", "logging", "datagen"]}
icu_provider_adapters = { version = "1.2.0", path = "../adapters" }
tinystr = { version = "0.7.1", path = "../../utils/tinystr", features = ["alloc", "serde", "zerovec"], default-features = false }
writeable = { version = "0.5.1", path = "../../utils/writeable" }
Expand Down