Allow constructing LevelFilter via incrementation
neithernut opened this issue · 9 comments
When implementing a command line application where logging is somewhat important (e.g. a longer-running service), I sometimes like to make the verbosity configurable via multiple sources such as configuration files, environment variables and command line options.
However, for the latter I usually want a somewhat different behavior. Rather than the number of -v (or --verbose) flags setting the verbosity directly, I want each -v to increase the level from whatever the baseline is, which may come from a config file or some default level.
Currently, this requires some awkward code in applications. If we had the possibility to create a LevelFilter from an integer, this would be trivial. This was proposed in the past but clearly rejected (#318, #460). However, I didn't yet find anyone proposing the possibility of creating a level by incrementing some LevelFilter.
That could be done by either:
- adding a new
fn increment(self)/fn (self)toLevelFilter, - a new
fn iter_from(self)toLevelFilter(whcih would be identical to the existingiter(self)and save users at least some awkward filtering), - implementing
std::ops::Add<usize>and/orstd::ops::AddAssign<usize>forLevelFilteror - implementing the currently unstable
std::iter::StepforLevelFilter.
The latter clearly needed to be feature gated (which would be awkward) and/or increase the MSRV, but would allow using LevelFilter with std::ops::Ranges as an Iterator in the future. And of course, those options are not mutually exclusive. The std::iter::Step impl can follow at some later point if/when it gets stabilized.
You can use min((level_filter as usize) + 1, Trace::Trace as usize) as LevelFilter as LevelFilter has #[repr(usize)] to do this already.
I'm not sure this a common enough case that we need the crate, though.
Are you sure? All versions of Rust which I tried this on say that the cast from usize to LevelFilter is illegal1. Given that the representation is known you can still hack the equivalent of C++'s reinterpret_cast, but...
Footnotes
-
I would provide a link to https://play.rust-lang.org, but it appears to be broken right now. ↩
This works: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1f583a914a1b153a80213f3cd78c8676 and it's safe only because of #[repr(usize)]. However you have to make sure the value is valid, i.e. not bigger than 5/LevelFilter::Trace, but the transmute itself is not UB.
Agreed, but it would still be nice not having to resort to unsafe, even if it is factually safe.
Agreed, but it would still be nice not having to resort to
unsafe, even if it is factually safe.
Not really, the compiler can't ensure you do 6 as LevelFilter, which would be UB (5 is max valid value).
Agreed, but it would still be nice not having to resort to
unsafe, even if it is factually safe.Not really, the compiler can't ensure you do
6 as LevelFilter, which would be UB (5 is max valid value).
Yes, I know. I meant that with an API like TryFrom<usize> or the ones I suggested in my initial post would remove the need for library users to use unsafe code like the one you posted earlier.
Which brings us back to
I'm not sure this a common enough case that we need the crate, though.
I agree that some kind of increment/decrement on log levels and/or filters would be useful. It would also circumvent the stated reasons the other issues were rejected (#318, #460). Namely, it doesn't increase exposure of the underlying integer representation to users. It still does, however, expose the exact sequence of elements, e.g. adding a new level between two existing levels might break someone's code. I don't know if that is an issue.
It still does, however, expose the exact sequence of elements, e.g. adding a new level between two existing levels might break someone's code. I don't know if that is an issue.
The sequence is already exposed via the Iterator returned by LevelFilter::iter. A new feature for iterating over levels will thus at least not introduce this issue.