-
Notifications
You must be signed in to change notification settings - Fork 10
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
author-only
, suppress-author
, composite
#117
Conversation
means using <intext> without activating the feature is a parse error. adds a hint for activating the feature.
fixes a couple of issues found also adds doctests for JSON parsing of clusters/cites
This is great work!
Would it be possible for
Since citeproc-rs API is incompatible in many ways with citeproc-js, I don't think it makes sense to maintain the compatibility here. Is there another reason for this (like test compatibility)? It would be much neater if the library maintained consistent camelCase across the board.
I am mostly completely uninformed on the citeproc test suite, but I wonder why this is failing. |
This is not really something I would make user-configurable, it's more of a CSL 1.1 spec problem that just needs a decision. As the citeproc-js docs are written I think my choice might be wrong, it should probably work anywhere because why have the limitation at all? I don't think it needs an urgent answer yet. Moreover, it is not possible to discover at parse time whether a style supports it, since by leading I mean "the first rendered" not the first to appear syntax-wise in the XML. The leading node can be as nested as you want under many groups, and all sorts of empty content can appear beforehand (e.g. choose blocks with no output) and it works just fine. What would be more helpful I think is having it configurable in the previewCitationCluster API. That way you can tick the box and see the result before saving the cluster in your document. Currently that doesn't work but I can punt it into a follow up PR if you want it.
Yes, the citeproc-js-format test suite code was able to be simplified by a small amount. But that's not super important, I can just add a second Cluster struct to the test suite again. I think I'll do that. While I'm at it, I'll probably change the Cite API to match the Cluster // only these two options for cites
let citeAO = { id: "jones2006", mode: "AuthorOnly" };
let citeSA = { id: "jones2006", mode: "SuppressAuthor" };
// additional settings for clusters
let clusterAO = { id: "one", cites: [...], mode: "SuppressAuthor" };
let clusterSA = { id: "one", cites: [...], mode: "SuppressAuthor" };
let clusterSA_First = { id: "one", cites: [...], mode: "SuppressAuthor", suppressFirst: 3 };
let clusterC = { id: "one", cites: [...], mode: "Composite" };
let clusterC_Infix = { id: "one", cites: [...], mode: "Composite", infix: ", whose book" };
let clusterC_Full = { id: "one", cites: [...], mode: "Composite", infix: ", whose books", suppressFirst: 0 };
I haven't figured out what I want to do about unauthored items. @fbennett chose to force them to emit (where [1] is cluster author-only, [2] is cluster suppress-author, [3] is cluster composite
and - lines are citeproc-rs, + lines are citeproc-js
[0] (Unauthored thing 1964)
-[1] Unauthored thing
-[2] (1964)
-[3] Unauthored thing (1964)
+[1] [NO_PRINTED_FORM]
+[2] (Unauthored thing 1964)
+[3] [NO_PRINTED_FORM] (Unauthored thing 1964) |
Small follow up on the first question: another reason why previewing would work better is that not every item works with it, even if the style as a whole has the potential to extract a non-empty author-only. Obviously this is because some things are unauthored. So you basically have to check every time anyway. |
Yep, at least for Zotero we definitely need to support all cite customizations in the preview call.
The example seems like abuse of the API, maybe. Maybe it makes sense to substitute the title for the author when no author is available, but that has to be documented and maybe discussed with the CSL people. |
To be clear the cite-level ones, e.g. While I'm at it breaking compatibility in favour of usability, I might also kill the reuse of the field name "id" to refer to every different kind of ID. |
Change is quite succinct overall, see IrTreeRef::leading_names_block_or_title +discretionary_ExampleUnauthored.txt
this brings -rs mostly in line with -js, I believe. adds test case whose output matches citeproc-js exactly. +intext_AuthorOnly_Title.yml +intext_AuthorOnly_DateYearSuffix.yml (to test a Substitute with a Seq inside it)
Latest changes bring -rs in line with -js on discretionary_ExampleUnauthored and some new tests I added. Now any names block or any output of |
Fixes #114, available in
@citeproc-rs/[email protected]
See the updated
crates/wasm/README.md
for JS-oriented documentation (diff). I made itciteproc-js
-compatible, which has unfortunately muddied the whole kebab-case vs camelCase situation. At least now Cite and Cluster are compatible with the CSL-JSON schemas.There are some minor incompatibilities with citeproc-js; the only divergent tests from the relevant citeproc-js
fixtures/local
tests are:discretionary_ExampleUnauthored.txt
, which remains failing.discretionary_AuthorOnly.txt
has a snapshot reflecting what I think is an inconsistency withdiscretionary_SuppressAuthor
, where AuthorOnly considers a date (issued) to be an author to be extracted, but SuppressAuthor does not suppress it.Other implementation notes:
<intext>
element has been feature flagged. Add<features><feature name="custom-intext" /></features>
to use, or force-enable that feature globally via Driver.new as described in the wasm README.Internals:
NodeId
+ owned/borrowed/mutableIrArena
. All the IR transforms are moved to functions defined on these types.LayoutStream
csl::Features
API including a way to specify enabled-by-default CSL features in the processor's initialisation options.crates/citeproc/tests/test_format