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

Use serde #260

Closed
gavrilikhin-d opened this issue May 2, 2023 · 4 comments
Closed

Use serde #260

gavrilikhin-d opened this issue May 2, 2023 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@gavrilikhin-d
Copy link
Contributor

gavrilikhin-d commented May 2, 2023

Could you make your SourceOffset, SourceSpan, Severity, LabledSpan and other types to implement Serialize and Deserialize?
Now you need to use builtin types instead of them to have serialization autoimplemented:

#[derive(Debug, Error, Diagnostic, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[error("expected '{expected}'")]
pub struct Expected {
    pub expected: String,
    #[label("{expected}")]
    pub at: usize, // This is auto serialized/deserialized
}

So you can't just write:

#[derive(Debug, Error, Diagnostic, Serialize, Deserialize, PartialEq, Eq, Clone)]
#[error("expected '{expected}'")]
pub struct Expected {
    pub expected: String,
    #[label("{expected}")]
    pub at: SourceOffset, // Doesn't work. Need to implement manually
}
@kleinesfilmroellchen
Copy link
Contributor

Since serde is a large dependency, this should most definitely be an optional or opt-out dependency for anyone who doesn't need it.

@zkat zkat added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels May 5, 2023
@zkat
Copy link
Owner

zkat commented May 5, 2023

This seems reasonable, as long as it's hidden behind a non-default serde flag. I can't really do a lot of stuff myself right now, but I probably have time to review/merge a PR if someone contributes it.

@gavrilikhin-d
Copy link
Contributor Author

Ok, I will create a PR on these weekends

@gavrilikhin-d
Copy link
Contributor Author

This one looks related: #245

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants