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

Serialization failure when PersonRole::Unknown is involved #266

Open
chenlijun99 opened this issue Jan 11, 2025 · 2 comments
Open

Serialization failure when PersonRole::Unknown is involved #266

chenlijun99 opened this issue Jan 11, 2025 · 2 comments

Comments

@chenlijun99
Copy link

The following code

// path is path to the biblatex file
let contents = fs::read_to_string(path)?;
let bib = hayagriva_io::from_biblatex_str(&contents).unwrap();

Ok(serde_json::to_value(bib).unwrap())

errors out with:

called `Result::unwrap()` on an `Err` value: Error("the enum variant PersonRole::Unknown cannot be serialized", line: 0, column: 0)

I believe this is caused by a wrong use of #serde(skip) in the PersonRole enum.
https://github.com/chenlijun99/hayagriva/blob/af5c146265b797a74edcb0c760e52e41834a037a/src/types/persons.rs#L83-L85

A similar issue was raised in serde: serde-rs/serde#2217 (comment).
The gist seems that the value itself cannot decide to be ignored.

Can be reproduced using the following biblatex entry.

@video{wachowskiMatrix1999,
  type = {Action, Sci-Fi},
  entrysubtype = {film},
  title = {The {{Matrix}}},
  editor = {Wachowski, Lana and Wachowski, Lilly},
  editortype = {director},
  editora = {Wachowski, Lilly and Wachowski, Lana},
  editoratype = {scriptwriter},
  namea = {Reeves, Keanu and Fishburne, Laurence and Moss, Carrie-Anne},
  nameatype = {collaborator},
  date = {1999-03-31},
  publisher = {Warner Bros., Village Roadshow Pictures, Groucho Film Partnership},
  abstract = {When a beautiful stranger leads computer hacker Neo to a forbidding underworld, he discovers the shocking truth--the life he knows is the elaborate deception of an evil cyber-intelligence.},
  keywords = {artificial reality,dystopia,post apocalypse,simulated reality,war with machines},
  annotation = {IMDb ID: tt0133093\\
event-location: United States, Australia}
}
chenlijun99 added a commit to chenlijun99/hayagriva that referenced this issue Jan 11, 2025
@PgBiel
Copy link
Contributor

PgBiel commented Feb 4, 2025

I think this was probably added to ensure persons with unknown roles wouldn't be serialized back into the hayagriva yaml format, but I think this might have been inappropriately handled. In theory, that bibliography cannot be turned into valid hayagriva, as there is a fixed list of allowed roles in the format specification, but maybe we could just flexibilize this restriction, at the cost of breaking some forwards-compatibility. Thoughts? cc @reknih

@reknih
Copy link
Member

reknih commented Feb 4, 2025

I'd rather not allow any value in Hayagriva person roles. I think that the error message that your person was not in the list is valuable, because CSL styles will not select or format arbitrary roles. I also see the need for a generic, catch-all role. Hence, I'd propose changing PersonRole::Unknown(String) to PersonRole::Other and removing its #[serde(skip)]

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

No branches or pull requests

3 participants