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

A few simple CSL fixes #2228

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Omikhleia
Copy link
Member

@Omikhleia Omikhleia commented Feb 15, 2025

So as to make review easier, here are several fixes before I tackle with enhancements.1

  • fix(packages): Empty CSL macros must be suppressed = Mentioned in passing in Can't run either \printbibliography or \csl:reference.  #2202 (comment)

    Some macros are interpreted as present while they should apparently be considered as absent. An example is the "translator" macro, which generates a "Tradução " even in the absence of translator(s) afterwards. Re-reading the CSL 1.0.2 specification, however, the only place where content suppression is clearly mentioned is on cs:group (acting as implicit conditional), but there's no such element here... It's still unclear to me where the intended behavior is defined...

    The same entries have the macro suppressed by citeproc-js, and it sounds logical (though poorly documented).

  • fix(packages): Invalid link on CSL DOI, PMID, PMCID with affixes = Also mentioned in the above-mentioned discussion:

    There's an interesting problem with links (with URL, DOI, etc.) and here again the CSL specification is somewhat obscure and I'd even say rather inconsistent (conflating usage for affixes...)

  • fix(packages): Error handling the locator on some CSL styles = Not reported before, but MLA for instance was affected (and likely a bunch of others, we were just lucky Chicago styles have an explicit condition). Just some bad code ;)

  • fix(packages): Correctly honor affixes on multiple CSL citations = Not reported before, but preparing the way to support citing multiple keys in our package, I realized the logic was wrong. The CSL specs are so unclear at places.

Footnotes

  1. Note to @jodros: The two first fixes correspond to issues I saw after fixing an (unrelated) issue of yours. This being said, there are still some weird things with UFES-ABNT, UNEAL-ABNT or ABNT-IPEA, notably places where there is either repeated punctuations (e.g. Lorem. . Something) or missing spacing (e.g. (Some editors, EDS.)Some collection title). However, I see the same issues on these entries with that JavaScript-based online BiBTeX Converter, so I am unsure it's really our fault.

@Omikhleia Omikhleia requested a review from alerque as a code owner February 15, 2025 13:14
@Omikhleia Omikhleia marked this pull request as draft February 15, 2025 13:15
@Omikhleia Omikhleia self-assigned this Feb 15, 2025
@Omikhleia Omikhleia added the bug Software bug issue label Feb 15, 2025
@Omikhleia Omikhleia added this to the v0.15.10 milestone Feb 15, 2025
@Omikhleia Omikhleia marked this pull request as ready for review February 15, 2025 19:30
@Omikhleia Omikhleia changed the title CSL fixes A few simple CSL fixes Feb 17, 2025
@alerque alerque assigned alerque and unassigned Omikhleia Feb 20, 2025
@alerque
Copy link
Member

alerque commented Feb 20, 2025

I'm not yet sure why CI has blown up, but that's probably on me to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants