-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Reason for citation order in bugreports_ChicagoAuthorDateLooping? #66
Comments
You're not missing anything -- not sure what's going on in the test fixture, which seems to be about disambiguation rather than sort order. |
If current Zotero/citeproc-js renders Manstein first then any objection if I submit a PR to change the test accordingly? As you note, the point of the test isn't sorting so I think it's worth having it runnable for those other reasons. |
@fbennett -- is that test passing for citeproc-js in your tests currently? If so, any idea why? |
I was stuck in the same problem until I read the code of citeproc-test-runner. The citation order is taken from the registry after |
Haven't looked at the code, but that sounds right. It would mean that the
test result should be amended, and the (current) citeproc-js test runner
should fail?
…On Fri, Jun 23, 2023, 01:57 Zeping Lee ***@***.***> wrote:
With tests like this that don’t have an explicit CITATION-ITEMS or
CITATIONS section, I’ve always taken the INPUT order to be the citation
order, then applied the specification, “In the absence of cs:sort, cites
and bibliographic entries appear in the order in which they are cited.”
The test *does* have a *bibliography* sort that creates the expected
order but I don’t know why that would affect citation order in this case.
What am I missing?
I was stuck in the same problem until I read the code of
citeproc-test-runner <https://github.com/Juris-M/citeproc-test-runner>.
The citation order is taken from the registry after updateItems() which
preforms the bibliography sorting.
https://github.com/Juris-M/citeproc-test-runner/blob/b1e72d5cb1363b7f4abbe1f6546c9e2c443db726/lib/sys.js#L369-L376
—
Reply to this email directly, view it on GitHub
<#66 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAASMSQGO7JDLRJTVCGINSLXMR2OLANCNFSM6AAAAAAZQIWLFE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks @zepinglee that makes sense. So yes to this
as well as to this
|
The fixture disambiguate_InitializeWithButNoDisambiguation.txt is also involved in this behavior. If the citation order is directly taken from the INPUT, the result will be |
Good work.
…On Fri, Jun 23, 2023, 09:51 Zeping Lee ***@***.***> wrote:
The fixture disambiguate_InitializeWithButNoDisambiguation.txt
<https://github.com/citation-style-language/test-suite/blob/759e6fd0e1ba2a7d11489e54062a85c57a42bb63/processor-tests/humans/disambiguate_InitializeWithButNoDisambiguation.txt>
is also involved in this behavior. If the citation order is directly taken
from the INPUT, the result will be (Doe, 1965b, 1965a) rather than (Doe,
1965a, 1965b).
—
Reply to this email directly, view it on GitHub
<#66 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAASMSWYFJ3QJO63BMJIVXLXMTR7TANCNFSM6AAAAAAZQIWLFE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@zepinglee, I understand from your comment that a test's citation order does, in some cases, follow the bibliography order. So a PR to change these tests (including the other test you note) will cause, as @fbennett mentions, the current citeproc-js test runner to fail. The alternative, which wouldn't take too much code now that I see the issue more clearly, would be to apply these rules:
This isn't my favorite alternative (will muddy up my code a bit) but I hesitate to suggest a PR that breaks long-standing existing code. |
Yes, that's correct. Also note that current citeproc-test-runner performs
I agree. It's a better solution if @fbennett would like to synchronously modify citeproc-test-runner and the tests. |
This is a citation test that expects
However, Manstein is first in the
INPUT
section and<citation/>
has no<sort/>
to change that so I render it asWith tests like this that don’t have an explicit
CITATION-ITEMS
orCITATIONS
section, I’ve always taken theINPUT
order to be the citation order, then applied the specification, “In the absence ofcs:sort
, cites and bibliographic entries appear in the order in which they are cited.”The test does have a bibliography sort that creates the expected order but I don’t know why that would affect citation order in this case.
What am I missing?
The text was updated successfully, but these errors were encountered: