Expose logging levels #459

2021-08-27T06:39:22-05:00

WaDelma commented

2021-08-27 06:39:22 -05:00

(Migrated from github.com)

I am using FromStr implementation of LevelFilter to parse logging level given by user via command line flag. In case user gives invalid logging level I want to print list of allowed values. Currently I have to replicate LOG_LEVEL_NAMES array in my code and remember to keep it in sync (It is probably unlikely it changes that much though).

One question I have is that is there a reason it is static instead of const?

I am using `FromStr` implementation of `LevelFilter` to parse logging level given by user via command line flag. In case user gives invalid logging level I want to print list of allowed values. Currently I have to replicate `LOG_LEVEL_NAMES` array in my code and remember to keep it in sync (It is probably unlikely it changes that much though). One question I have is that is there a reason it is `static` instead of `const`?

KodrAus commented

2021-10-26 05:15:55 -05:00

(Migrated from github.com)

Hey @WaDelma 👋

There probably isn't a great reason for this to be static rather than const. Rather than make the array public though I think a static method on Level that returns some impl Iterator<Item = Level> might be better:

pub fn iter_all() -> impl Iterator<Item = Level> {
    ..
}

That way in your code you could call something like:

let names = Level::iter_all().map(|l| l.as_str()).collect();

The reason I think we should avoid making the array public is that it would be a breaking change to add any more levels (as unlikely as that is) because the type would change from [&'static str; 6] to [&'static str; 7].

Hey @WaDelma :wave: There probably isn't a great reason for this to be `static` rather than `const`. Rather than make the array public though I think a static method on `Level` that returns some `impl Iterator<Item = Level>` might be better: ```rust pub fn iter_all() -> impl Iterator<Item = Level> { .. } ``` That way in your code you could call something like: ```rust let names = Level::iter_all().map(|l| l.as_str()).collect(); ``` The reason I think we should avoid making the array public is that it would be a breaking change to add any more levels (as unlikely as that is) because the type would change from `[&'static str; 6]` to `[&'static str; 7]`.

👍 1

WaDelma commented

2021-10-27 10:00:23 -05:00

(Migrated from github.com)

Makes sense. I added such static methods for both Level and LevelFilter,

Makes sense. I added such static methods for both `Level` and `LevelFilter`,

KodrAus (Migrated from github.com) reviewed

2021-11-05 07:10:18 -05:00

src/lib.rs

					
				@ -563,0 +564,4 @@

				    /// Iterate through all supported logging levels

				    ///

				    /// The order of iteration is from more severe to less severe log messages

				    pub fn iter() -> impl Iterator<Item = Self> {

It might be worth documenting something about the order of these fields. Using from_usize we’re effectively returning them in priority order.

What do you think?

It might be worth documenting something about the order of these fields. Using `from_usize` we’re effectively returning them in priority order. What do you think?