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

feat: add citation element #120

Merged
merged 42 commits into from
Feb 2, 2024
Merged

feat: add citation element #120

merged 42 commits into from
Feb 2, 2024

Conversation

mhatzl
Copy link
Contributor

@mhatzl mhatzl commented Nov 20, 2023

This PR adds the citation element, and CSL handling to render citations according to a given CSL style.

Relevant decisions you made in this PR

Storing citation IDs in parser contexts

According to CSL, citations of the same IDs may differ depending on the citation order.
To correctly handle this, all citations are first stored in a citation list
that is stored in the BlockContext of the block parser.
Before rendering to an output format, the parsed citations are processed in-order by citeproc-js
to get the correct citation content.
The rendered citations are then passed as list to the respective renderer for the output format.

CSL locale files in preamble

CSL processors need locale information to correctly render citations.
The official CSL repository provides some locales, but not every locale is covered.
Therefore, the parameter citation_locales was added to allow adding custom locale files.

Copy link
Contributor Author

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

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

It is still a draft, so take my comments more like hints and suggestions.

Summary:

  • Added some remarks to reduce possible panics.
  • Suggested using more functionality provided by serde
  • Suggested workaround to get a HashMap for clap + serde

render/src/html/citeproc/csl_files.rs Outdated Show resolved Hide resolved
render/src/html/citeproc/csl_files.rs Outdated Show resolved Hide resolved
render/src/html/citeproc/csl_files.rs Outdated Show resolved Hide resolved
render/src/html/citeproc/mod.rs Outdated Show resolved Hide resolved
render/src/html/render.rs Outdated Show resolved Hide resolved
render/src/render.rs Outdated Show resolved Hide resolved
commons/src/config/mod.rs Outdated Show resolved Hide resolved
commons/src/config/locale.rs Outdated Show resolved Hide resolved
commons/src/config/locale.rs Outdated Show resolved Hide resolved
commons/src/config/locale.rs Show resolved Hide resolved
commons/src/config/locale.rs Outdated Show resolved Hide resolved
commons/src/config/mod.rs Outdated Show resolved Hide resolved
commons/src/config/preamble.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

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

looks good.
One minor change to use Option<String> to prevent "" from being used as None.

I tried to check the JS part, but I am not too familiar with JS.
Could not find any obvious errors, and the tests are passing, so I assume that its ok.

cli/tests/umi.rs Outdated Show resolved Hide resolved
commons/src/config/locale.rs Outdated Show resolved Hide resolved
commons/src/config/mod.rs Outdated Show resolved Hide resolved
render/src/render.rs Outdated Show resolved Hide resolved
render/src/render.rs Show resolved Hide resolved
render/src/render.rs Outdated Show resolved Hide resolved
@mhatzl mhatzl marked this pull request as ready for review January 30, 2024 19:51
@mhatzl mhatzl merged commit b133fb6 into main Feb 2, 2024
3 checks passed
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.

2 participants