tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.43k stars 93 forks source link

Equivalents of `serde_json::from_*` #2097

Open rben01 opened 3 days ago

rben01 commented 3 days ago

Is your feature request related to a problem? Please describe. It would be nice if nickel had functions that would wrap the entire deserialization pipeline. For instance, serde_json has from_str, from_reader, etc. In nickel, however, you have to create a program, manually set its source name and stderr, then eval it, and then deserialize.

Describe the solution you'd like Something like this (but without the nested Result; it's necessary in this example because AFAICT RustDeserializationError isn't Into<nickel_lang_core::error::Error>).

fn from_reader<'de, R, T>(
    r: R,
) -> Result<Result<T, RustDeserializationError>, nickel_lang_core::error::Error>
where
    R: std::io::Read,
    T: serde::Deserialize<'de>,
{
    Ok(T::deserialize(
        Program::<CacheImpl>::new_from_source(r, "source", std::io::stderr())
            .map_err(IOError::from)?
            .eval_full_for_export()?,
    ))
}

Describe alternatives you've considered None

Additional context Related: #1751

yannham commented 3 days ago

Thanks for reaching out, it's interesting for us to know how much people are using Nickel to deserialize directly from Rust. I agree that the current setup is workable but really not ideal (and I really don't like this CBNCache abstraction leaking everywhere).

What you propose is a simple and good first step. If you wish to take a stab at implementing it, we would accept the PR :+1:

rben01 commented 2 days ago

Happy to do a PR. What's the proper way to do error handling for this function? It should return Result<T, E>, not a Result<Result<T, E1>, E2>, but what's the correct type for E?

yannham commented 2 days ago

Indeed, nesting results isn't great.

From the top of my head, I suspect we'll need a new kind of error. The error hierarchy ends up with the most general error::Error type, that subsumes most other errors, but it doesn't subsume RustDeserializationError which is quite specialized - in particular because the latter isn't supposed to be rendered as a diagnostic but to be handled by the consumers of the Nickel lib, while error::Error must be renderable as nice diagnostics and thus must implement the IntoDiagnostics trait.

Naming is hard - so this is just a placeholder name I'm giving - but we need a new enum EvalOrDeserError { Eval(EvalError), Deser(RustDeserializationError) } in crate::error (or maybe we need to make it Eval(Error), as creating a program and running it might probably throw general errors - I haven't checked). At least that's how I would go.