Skip to content

Conversation

WolverinDEV
Copy link

Using env_logger in a no_std environment may not seem useful at first. However, the env_filter crate is very convenient for defining complex filters that can then be implemented by a custom no_std-compatible logger.

This approach leverages the convenience developers are already familiar with from env_logger, while enabling the same flexibility in no_std contexts with other logging backends.

Comment on lines +61 to +62
#[cfg(not(feature = "std"))]
impl core::error::Error for ParseError {}
Copy link
Contributor

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

Comment on lines +29 to +35
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 {
Copy link
Contributor

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?

Comment on lines +1 to +3
use alloc::string::{String, ToString};

use core::fmt;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you group these?

Comment on lines +1 to 10
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;
Copy link
Contributor

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

Comment on lines +1 to 10
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Comment on lines +34 to +35
log = { version = "0.4.8", default-features = false }
regex = { version = "1.0.3", optional = true, default-features = false, features = ["std", "perf"] }
Copy link
Contributor

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?

Copy link
Contributor

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

@coveralls
Copy link

Pull Request Test Coverage Report for Build 17143312963

Details

  • 1 of 3 (33.33%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 43.914%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/env_filter/src/filter.rs 1 2 50.0%
crates/env_filter/src/parser.rs 0 1 0.0%
Totals Coverage Status
Change from base Build 16804394720: 0.0%
Covered Lines: 267
Relevant Lines: 608

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants