Implement Serialization and Deserialization for Verbosity #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

epage merged 3 commits into clap-rs:master from joshka:jm/serde

Aug 19, 2025

Contributor

joshka commented Sep 20, 2024 •

edited

Loading

Verbosity is serialized and deserialized using the lowercase of the
VerbosityFilter (e.g. "debug")

The serde dependency is gated behind an optional feature flag.

Added conversion methods between Verbosity and VerbosityFilter to
simplify the implementation and derived PartialEq, Eq impls on
types where this was necesary for testing.

Fixes: #88

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems

View reviewed changes

src/serde.rs Fixed Show fixed Hide fixed

src/serde.rs Fixed Show fixed Hide fixed

src/serde.rs Fixed Show fixed Hide fixed

src/serde.rs Fixed Show fixed Hide fixed

src/serde.rs Fixed Show fixed Hide fixed

src/serde.rs Fixed Show fixed Hide fixed

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

coveralls commented Sep 20, 2024 •

edited

Loading

Pull Request Test Coverage Report for Build 17053845567

Details

8 of 12 (66.67%) changed or added relevant lines in 1 file are covered.
5 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.5%) to 49.541%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/lib.rs	8	12	66.67%

Files with Coverage Reduction	New Missed Lines	%
src/lib.rs	5	49.28%

Totals
Change from base Build 16806571281:	-0.5%
Covered Lines:	54
Relevant Lines:	109

💛 - Coveralls

epage reviewed

View reviewed changes

src/serde.rs Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

src/serde.rs Outdated Show resolved Hide resolved

joshka commented

View reviewed changes

Contributor Author

joshka left a comment

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 Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/serde.rs Outdated Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved

joshka mentioned this pull request

Use u8 for internal verbosity level calculation #115

Merged

joshka force-pushed the jm/serde branch from 4ff01a3 to 2dda2ab Compare

September 21, 2024 03:31

joshka commented

View reviewed changes

Contributor Author

joshka left a comment

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

Cargo.toml Outdated Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/serde.rs Outdated Show resolved Hide resolved

src/serde.rs Outdated Show resolved Hide resolved

joshka commented

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

joshka force-pushed the jm/serde branch from 2caa27f to 28f54e2 Compare

September 21, 2024 03:57

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

epage mentioned this pull request

Serialize & Deserialize #88

Closed

joshka force-pushed the jm/serde branch from 28f54e2 to e52cc3a Compare

September 26, 2024 19:02

github-advanced-security bot found potential problems

View reviewed changes

src/lib.rs Fixed Show fixed Hide fixed

src/lib.rs Fixed Show fixed Hide fixed

src/lib.rs Fixed Show fixed Hide fixed

joshka force-pushed the jm/serde branch 2 times, most recently from 2a3fb79 to 99d104e Compare

September 26, 2024 19:14

joshka force-pushed the jm/serde branch from 99d104e to eb58de2 Compare

November 29, 2024 01:22

github-advanced-security bot found potential problems

View reviewed changes

src/lib.rs Fixed Show fixed Hide fixed

joshka force-pushed the jm/serde branch from eb58de2 to 8ffd04c Compare

November 29, 2024 01:23

Contributor Author

joshka commented Nov 29, 2024

In eb58de2, I rewrote this based on the 3.0 release. Notably this uses the VerbosityFilter to de/serialize instead of log::LogLevelFilter which means that it only handles Title case deserialization. I've also simplified the style and quantity of tests significantly.

joshka force-pushed the jm/serde branch from 8ffd04c to b84256c Compare

November 29, 2024 01:31

epage force-pushed the jm/serde branch from b84256c to f5da5c6 Compare

December 6, 2024 20:35

epage reviewed

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

Cargo.toml Show resolved Hide resolved

Lurk commented Aug 16, 2025

I need that Serialise/Deserialize functionality and would like to continue work on it.

As far as I understand, the way to go is to have three separate PRs:

PR for how the count math is done
PR for impl<L: LogLevel> From<Verbosity<L>> for VerbosityFilter and impl<L: LogLevel> From<VerbosityFilter> for Verbosity<L> (this one will also include derive Eq for tests)
PR for feature-gated Serialize/Deserialize

Does it sound ok?

Lurk mentioned this pull request

Config as a file Lurk/serve#10

Closed

joshka force-pushed the jm/serde branch from f5da5c6 to 162d6ca Compare

August 16, 2025 21:40

Contributor Author

joshka commented Aug 16, 2025

As far as I understand, the way to go is to have three separate PRs:

Likely just massaging the commits in this PR would do fine here. Ed has a high bar for small documented commits. Above force push is just a rebase onto the current main. Splitting in process now.

joshka force-pushed the jm/serde branch from 162d6ca to a56a155 Compare

August 16, 2025 21:55

joshka commented

View reviewed changes

src/lib.rs Show resolved Hide resolved

joshka force-pushed the jm/serde branch from a56a155 to e52f3b4 Compare

August 16, 2025 21:58


          feat: Add Eq, and PartialEq impls to various types

e6f4e62

joshka force-pushed the jm/serde branch from e52f3b4 to efa6eee Compare

August 16, 2025 22:58


          feat: Add conversion methods between Verbosity and VerbosityFilter

f735876

This will simplify the implementation of Serialization.

joshka force-pushed the jm/serde branch from efa6eee to 3165fca Compare

August 16, 2025 22:58

This comment was marked as duplicate.

Sign in to view

joshka force-pushed the jm/serde branch from 3165fca to f27a82c Compare

August 18, 2025 19:57

Contributor Author

joshka commented Aug 18, 2025

Updated to serialize and deserialize from lowercase.

epage reviewed

View reviewed changes

.vscode/settings.json Outdated Show resolved Hide resolved


          feat: Implement Serialization and Deserialization

44f0caa

Verbosity is serialized and deserialized using the title case of the
VerbosityFilter (e.g. "Debug")

The `serde` dependency is gated behind an optional feature flag.

Fixes: clap-rs#88

joshka force-pushed the jm/serde branch from f27a82c to 44f0caa Compare

August 18, 2025 22:14

epage merged commit 210e8d9 into clap-rs:master

17 checks passed

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

Labels

None yet