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

Added from_* functions for easy deserialization for crates using nickel as a lib #2099

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rben01
Copy link
Contributor

@rben01 rben01 commented Nov 19, 2024

Closes #2097

Copy link
Member

@jneem jneem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, this is certainly a much nicer public API. I just have a couple of requests around error handling.

core/src/deserialize.rs Outdated Show resolved Hide resolved
@@ -30,6 +35,70 @@ macro_rules! deserialize_number {
};
}

#[derive(Debug, PartialEq)]
pub enum EvalOrDeserError {
Nickel(crate::error::Error),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a Nickel { error: crate::error::Error, files: crate::files::Files } would be better here, since it would allow the caller to print nice diagnostics if they want to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble figuring out the right way to turn an Input into a Files. When constructing EvalOrDeserError from a crate::error::Error, what should files look like? It seems like there is a bit of a chicken and egg problem, where constructing Files requires reading a file, which can fail, which can return the very error we're trying to construct...

Copy link
Member

@jneem jneem Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get the Files from the Program, like here

(edit: the reason you can't get Files directly from the input is that Files can include more things, like imports that were discovered while interpreting the original input)

Copy link
Contributor

github-actions bot commented Nov 20, 2024

Changed `from_input` to write to `NullReporter`
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.

Equivalents of serde_json::from_*
2 participants