-
Notifications
You must be signed in to change notification settings - Fork 142
feat: Adding support for no_std
environments for env_filter
#378
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
base: main
Are you sure you want to change the base?
Conversation
#[cfg(not(feature = "std"))] | ||
impl core::error::Error for ParseError {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes the lack of std
require a higher MSRV
pub(crate) fn new(spec: &str) -> Result<Self, String> { | ||
Ok(Self { | ||
inner: spec.to_string(), | ||
}) | ||
} | ||
|
||
pub fn is_match(&self, s: &str) -> bool { | ||
pub(crate) fn is_match(&self, s: &str) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you split this out into a separate commit?
use alloc::string::{String, ToString}; | ||
|
||
use core::fmt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you group these?
use alloc::borrow::ToOwned; | ||
use alloc::format; | ||
use alloc::string::String; | ||
use alloc::vec::Vec; | ||
|
||
use core::fmt::{Display, Formatter}; | ||
use log::LevelFilter; | ||
use std::error::Error; | ||
use std::fmt::{Display, Formatter}; | ||
|
||
use crate::Directive; | ||
use crate::FilterOp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're changing grouping, could you do
use {core,alloc,std}
use third-party
use crate
use alloc::borrow::ToOwned; | ||
use alloc::string::ToString; | ||
use alloc::vec::Vec; | ||
|
||
use core::fmt; | ||
use core::mem; | ||
|
||
use log::{LevelFilter, Metadata, Record}; | ||
|
||
use crate::enabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
log = { version = "0.4.8", default-features = false } | ||
regex = { version = "1.0.3", optional = true, default-features = false, features = ["std", "perf"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this belongs in the previous comit, could you move it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor(filter): Add support for no_std environments
Mind fixing your commit type?
A refactor, by definition, has no user facing changes while this does. I would use feat
Pull Request Test Coverage Report for Build 17143312963Details
💛 - Coveralls |
Using
env_logger
in ano_std
environment may not seem useful at first. However, theenv_filter
crate is very convenient for defining complex filters that can then be implemented by a customno_std
-compatible logger.This approach leverages the convenience developers are already familiar with from
env_logger
, while enabling the same flexibility inno_std
contexts with other logging backends.