Skip to content
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.

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

Flexible inputs, second attempt #100

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

nmathewson
Copy link
Contributor

This is a second attempt to implement the idea of #97.

Instead of changing the default behavior of impl Deserialize for *, it instead provides new extract_lossy and extract_inner_lossy methods that implement the lossy behavior across the entire figment object.

(As an alternative, as discussed on #97, the user might annotate all their booleans and numbers with deserialize_with. But it can be error-prone to do so, especially if the configuration structure is large and complex.)

@ijackson
Copy link

ijackson commented Apr 3, 2024

Hi. I'm one of @nmathewson's colleagues. I mostly just wanted to say thanks to @SergioBenitez for engaging with this, to try to help meet our needs. @nmathewson, I'd be happy to help here. Eg, would you like me to read the code in this MR?

I did have one question. Why are we calling these methods "lossy"? I hope they aren't really lossy. But I just came in here so maybe I am confused...

@SergioBenitez
Copy link
Owner

I really like this idea, and I'm committed to reviewing/merging this as soon as I free up a bit (in the next few days)!

@ijackson Thank you! I chose "lossy" because we lose the true type information in the conversion process. In this comment I show how this is truly a loss and not just a conversion.

@nmathewson
Copy link
Contributor Author

nmathewson commented Apr 9, 2024

@ijackson Thank you! I chose "lossy" because we lose the true type information in the conversion process. In #97 (comment) I show how this is truly a loss and not just a conversion.

I do agree that when we consider these conversions as applied to individual strings or numbers, they are lossy: When you convert "yes" to true, you can't recover the original "yes" if it turns out that what you really wanted was a string.

But I wonder whether "lossy" is the right word for the overall feature when applied to an entire configuration in extract_lossy. When we convert "yes" to true in the context of a boolean option, we aren't losing any information, any more than you we are when we treat 1.0 and 1.00 as the same float.

(Edited to add: That said, having this option is more important to me than having the perfect name for this option. :) )

@SergioBenitez SergioBenitez force-pushed the flexible_serde_v2 branch 3 times, most recently from 7fb9479 to 410852a Compare April 18, 2024 00:19
This commit adds `Figment::extract_{inner_}lossy()`, variants of the
existing methods that convert string representations of booleans and
integers into their boolean and integer forms. The original string form
is lost and is not directly recoverable.

Methods that performs the same conversion are added to `Value` types:

  * `Value::to_num_lossy()`
  * `Value::to_bool_lossy()`
  * `Num::to_u128_lossy()`
  * `Num::from_str()`

Closes SergioBenitez#97.

Co-authored-by: Sergio Benitez <sb@sergio.bz>
@SergioBenitez SergioBenitez merged commit 2c83ffa into SergioBenitez:master Apr 18, 2024
8 checks passed
@SergioBenitez
Copy link
Owner

Published in 0.10.18! Thanks for working with me on this. Fantastic work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants