Restructure Hayagriva as a Workspace - #489
Conversation
Has it been decided that If I understand correctly, it will replace the arrow Bib(La)TeX So adding Footnotes
|
|
I see the potential for discrepancies between biblatex and Hayagriva but I still think this is the right course for two reasons:
My hope is -- and I will test this -- that Hayagriva's and biblatex's data structures for numbers, persons, etc. are not so different and that we can share code to a large extent. In my experience, the hard part here are not the data types but the different entry and field types. If you don't agree or have any other thoughts, I'd be happy for your input. |
|
@laurmaedje This PR is ready for input. It's still a draft but if you have any feedback on the structure/approach, let me know. |
if this means you are aiming for a good bib(la)tex parser in rust, I am very interested. I have tried to get a few rust-based parser projects to the point they can cleanly parse the BBT test suite but that got kludgy very fast and I stranded on each. BBT builds an CST-ish of the input before it applies the myriad of arcane rules of (bib)latex that express how the input should be handled and I couldn't get that into any of them. I would love to say goodbye to unified-latex. BBTs parser is a separate project BTW, at https://github.com/retorquere/bibtex-parser/ |
|
For parsing of bib(la)tex we will still rely on typst/biblatex. But I really should test that against the BBT test suite. I expect I'll have to take a closer look at BBT's parser. |
|
That makes sense of course. But I think that was one of the projects I tried to adapt. |
|
Fair enough. I'll give the test suite a try and then pick your brain for ideas (also for #497). |
|
The BBT parser has a test suite that just does field-by-field parsing/transform and doesn't do further transformation to another format, so it should be fairly easy to test against. I am open to the possibility that there are testcases that turn out to be wrong, but by and large, I think it is good and representative. Whether you think all cases should be catered for is another matter -- the parser handles some cases that are uncommon occurrences. I find the added complexity worthwhile, but that is definitely a value judgement. That parser has a few modes for recalcitrant users that do not want sentence-casing or case protection; it also transforms markup to the limited subset of HTML that Zotero supports. Open to changing that as long as I can keep targeting Zotero. |
laurmaedje
left a comment
There was a problem hiding this comment.
Hey, I managed to take a look now. I like the overall direction of the multi-crate structure, but have some remarks. :)
Crate structure and public APIs
-
I would like to see the public APIs tightended to the minimum that's necessary. This makes them clearer to use and easier to evolve without having public-facing breaking changes.
Here are some examples:
LOCALEScould be private since the public entry point islocales- For
hayagriva-core, it's unfortunate to have a publictest_utilmodule hayagriva-formathas empty publicinteropandtaxonomymodules and thecitation_labelmodule has just one Item. Maybe that isn't worth a module and could be reexported at the root or moved.
In essence, I would try to have a more deliberatly crafted public API that does not necessarily just reflect the source code structure, but aims to be most easy to use and understand for a consumer of the crate.
Sometimes, definitions are needed in other crates but not really supposed to be used by consumer. I'm primarily thinking of
hayagriva_core::utils. We could consider making such thingsdoc(hidden)and semver excempt.(The trade-off here is also a bit different than for Typst, I would say, as the compiler APIs are supposed to be permanently unstable, while here we could potentially want to release a stable version at some point.)
-
It feels a bit strange that
hayagriva_format(which I would think should be CSL-independent) depends on and re-exportscitationberg. Would it make sense to move the taxonomy stuff intohayagriva-csland make it depend onhayagriva-format? -
On that topic: The split between
hayagriva-coreandhayagriva-formatis not 100% clear to me. E.g.PersonsWithRolesseems to only be used byhayagriva-formatand could reasonably live there? -
The crate name
hayagriva_jsonis a bit generic as the crate is CSL-specific. From the purpose, I would rather expecthayagriva_csl_json. But perhaps this isn't worth its own crate and can just be folded intohayagriva_csl? -
Is
hayagriva-biblatexsupposed to stay empty in this PR or is it supposed to be populated still? If it is supposed to stay empty, I would maybe just introduce it in a later PR. -
I'm wondering what will happen to the existing main
hayagrivacrate? Just deprecate it or would it make sense to have this in some way as an umbrella crate?
Making it work with crates.io
There are two issues which will cause issues once we release to crates.io.
-
Including the archives from outside the crate root of
hayagriva-archivewill not work when the crate is published to crates.io. We would need to move the archive files into that crate. -
The current path dependencies will not work with crates.io. Instead of
hayagriva-core = { path = "../hayagriva-core" }, I would usehayagriva-core = { workspace = true }and at the workspace root addhayagriva-core = { path = "crates/hayagriva-core", version = ".." }. You can see this pattern in action intypst/typst.
Tests
To me, the current nested tests/tests structure is a bit confusing. Also, there is no need for empty tests/src/lib.rs. I would eliminate one layer of nesting and I would also lift that folder out of crates to the top-level. You can see an example of just this pattern in typst/typst.
Nits
Finally, just some nits. I expect that some of these are just not addressed yet because we want to settle the broad picture first, but I thought I'd just mention everything I noticed for completeness.
- It would be nice to have docs on all types that are visible on any of the crates via
cargo doc --workspace - It would be also be nice if all crates had a description (Side note: The description for hayagriva-biblatex is currently wrong)
- Why did the
sv-SElocale change? If there was an intentional locale update, maybe it would be better to keep it separate from this PR - You have set
keywords.workspace = truein the crates, but deleted the top-level keywords array - The workspace members array can use globs such as
"crates/*" - The
extern cratedeclarations are not necessary - There are some cases where the standard import order (std / extern / local blocks separate by blanks) is not respected. Also, we generally don't use nested imports for modules in our style (unfortunately not enforcable via rustfmt on stable)
This PR reworks Hayagriva to be a workspace with the different parts of Hayagriva split into separate crates.
The crates and their purposes are:
EntryLikeimplementation for biblatex'sEntry)Entryand other definitions for the Hayagriva format (and, right now, the translation from biblatex to Hayagriva)EntryLikeimplementation for itBuilding on this, we can achieve directly going from biblatex or CSL-JSON to the rendering stage without going through the (lossy) Hayagriva translation steps. Users can also include just the parts of Hayagriva they actually need.
This is still a draft. The TODOs are:
TODOs by @laurmaedje:
LOCALEStest_utilincoreformatmodulesutilsincoreformatintocsland make it depend onformat?hayagriva-coreandhayagriva-format. E.g.PersonsWithRoles.hayagriva_jsontohayagriva_csl_json.hayagriva-biblatexfor now.hayagrivacrate? Just deprecate it or would it make sense to have this in some way as an umbrella crate?"crates/*"globextern crate