Implement Serialization and Deserialization for Verbosity #114

joshka · 2024-09-20T11:39:38Z

This commit adds serialization and deserialization support for the
Verbosity type. The verbosity is serialized as an optional log level
that represents the equivalent log level given the number of verbose and
quiet flags. When deserializing, the log level is converted into a
number of verbose and quiet flags, which keeps one of the flags at 0 and
the other at the difference between the log level and the default log
level.

The internal representation in calculations has been changed from i8 to
i16 to prevent overflows when performing calculations using u8 values.

Fixes: #88

src/lib.rs

src/serde.rs

src/lib.rs

coveralls · 2024-09-20T14:17:58Z

Pull Request Test Coverage Report for Build 11058763842

Details

14 of 17 (82.35%) changed or added relevant lines in 1 file are covered.
4 unchanged lines in 1 file lost coverage.
Overall coverage increased (+1.5%) to 68.182%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/lib.rs	14	17	82.35%

Files with Coverage Reduction	New Missed Lines	%
src/lib.rs	4	68.18%

Totals
Change from base Build 11057896653:	1.5%
Covered Lines:	45
Relevant Lines:	66

💛 - Coveralls

src/serde.rs

Cargo.toml

src/serde.rs

joshka

I will:

remove rstest
serialize to/from LevelFilter instead of Option(Level)
add a specific test for TOML serialization
Fix the github lints
split the i8 change out to another commit (and will raise another PR for it if necessary)

Cargo.toml

src/lib.rs

src/serde.rs

Cargo.toml

joshka

Moving to LevelFilter made it easy to derive serialize / deserialize using From<LevelFilter> instead of a custom implementation.

Cargo.toml

src/lib.rs

src/serde.rs

src/lib.rs

epage · 2024-09-26T01:54:09Z

src/lib.rs

+        }
+
+        // round-trips
+        let toml = "meaning_of_life = 42\nverbose = \"DEBUG\"\n";


Not sure how I feel about deserializing to all-caps

Yeah, this irks me a little too, but being consistent with the existing behavior won out over my ideal approach. If I was designing the log crate, I think I'd have made that serialization lower case too.

Here, this can deserialize from either case, but serializes to upper case. It comes from the serialization of log::LevelFilter. That means upper case is consistent with any other serialization that works with log level serialization (and is consistent with the usual display of the level in log outputs).

If you did want to avoid this, it would require going back to using a custom serialization implementation. This doesn't seem worth the extra code and tests to me. I prefer the simplicity even if that comes with a slightly ugly default which is consistent with other things doing the same thing.

I also suspect the serialization side of this will be used much less frequently than the deserialization side, and the serialization side will also often be more focused on programs re-reading the format rather than humans.

On one hand, if people are serializing LevelFilter, this keeps us consistent with that. On the other hand, this will produce results inconsistent with every other serialized value. This isn't just important here but for other cases like schemars. I think I want to let this sit for a time to mull over this.

no problem :)

src/lib.rs

This commit adds serialization and deserialization support for the Verbosity type. The verbosity is serialized using the log::LevelFilter enum that represents the equivalent number of verbose and quiet flags. The serialized value is the uppercase variant of the enum variant. Deserialing is case-insensitive. Fixes: clap-rs#88