log icon indicating copy to clipboard operation
log copied to clipboard

Allow constructing LevelFilter via incrementation

Open neithernut opened this issue 1 year ago • 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) to LevelFilter,
  • a new fn iter_from(self) to LevelFilter (whcih would be identical to the existing iter(self) and save users at least some awkward filtering),
  • implementing std::ops::Add<usize> and/or std::ops::AddAssign<usize> for LevelFilter or
  • implementing the currently unstable std::iter::Step for LevelFilter.

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.

neithernut avatar Mar 29 '24 14:03 neithernut

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.

Thomasdezeeuw avatar Mar 29 '24 14:03 Thomasdezeeuw

Are you sure? All versions of Rust which I tried this on say that the cast from usize to LevelFilter is illegal[^1]. Given that the representation is known you can still hack the equivalent of C++'s reinterpret_cast, but...

[^1]: I would provide a link to https://play.rust-lang.org, but it appears to be broken right now.

neithernut avatar Mar 29 '24 15:03 neithernut

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.

Thomasdezeeuw avatar Mar 29 '24 16:03 Thomasdezeeuw

Agreed, but it would still be nice not having to resort to unsafe, even if it is factually safe.

neithernut avatar Mar 29 '24 16:03 neithernut

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).

Thomasdezeeuw avatar Mar 29 '24 16:03 Thomasdezeeuw

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.

neithernut avatar Mar 29 '24 16:03 neithernut

Which brings us back to

I'm not sure this a common enough case that we need the crate, though.

Thomasdezeeuw avatar Mar 29 '24 16:03 Thomasdezeeuw

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.

Zoybean avatar Jun 02 '24 07:06 Zoybean

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.

neithernut avatar Jun 05 '24 14:06 neithernut

Solved by #692

neithernut avatar Sep 04 '25 08:09 neithernut