Skip to content

Conversation

@steinm
Copy link
Contributor

@steinm steinm commented Oct 15, 2025

This is based on the old PR 1324. It took into account the comments from Demian. The unit tests are not fully working, because the html output has changed. There is an extra diff around a bibliographie item and currently there is still the comparision between the old and new output.

It still uses seboettg/citeproc-php from https://github.com/seboettg/citeproc-php/ though I thing the fork at https://github.com/glorieux-f/citeproc-php has some useful improvements. Unfortanetly, it introduces a line of
code which requires at least php 8.3. The could easily be made compatible with 8.2, but I didn't want to start another fork.

TODO:

  • Add test records and custom import properties to fully exercise all the fields involved in citations, for easier/more comprehensive testing. This should include not just monographic items but also journal articles that utilize VuFind's "container" fields.
  • Update test class
  • Resolve VUFIND-889 when finished

@demiankatz
Copy link
Member

Thanks, @steinm, I appreciate the work on this! And I understand that the tests are broken -- probably best to finalize the functionality before spending a lot of time tweaking tests.

Do you think we should close #1324 so that we can focus on this new version? If so, should we carry forward some/all of the TODO checkboxes from there to here?

My capacity for reviewing PRs is a bit limited right now since I'm trying to focus on the upcoming 11.0 release, but I will look at this in more depth as soon as I have the bandwidth! :-)

@steinm
Copy link
Contributor Author

steinm commented Oct 15, 2025

Thanks, @steinm, I appreciate the work on this! And I understand that the tests are broken -- probably best to finalize the functionality before spending a lot of time tweaking tests.

Do you think we should close #1324 so that we can focus on this new version? If so, should we carry forward some/all of the TODO checkboxes from there to here?

Yes, I think #1324 can be closed. If taking over the checkboxes is helpful and a proven workflow, then we should do it.

My capacity for reviewing PRs is a bit limited right now since I'm trying to focus on the upcoming 11.0 release, but I will look at this in more depth as soon as I have the bandwidth! :-)

@demiankatz
Copy link
Member

Thanks, @steinm! I've carried over the old TODOs that seem most likely to be relevant here, and I have closed the old PR. I might add some more TODOs once I've had time to start reviewing and testing this!

@demiankatz demiankatz changed the title add CSL support for citations Add Citation Style Language (CSL) support for citations Oct 15, 2025
@ThoWagen
Copy link
Contributor

ThoWagen commented Oct 23, 2025

Just want to mention that the future of citeproc-php is not clear at the moment seboettg/citeproc-php#200

We are currently using it in our local customization, but because we had some problems with it and newer version of citation-style-language and don't have the capacity maintain it ourself, we decided to abandon it and implement some additional styles directly in upstream VuFind.

Not saying that we should not use it, but just wanted to make sure that you are aware, that it is currently not actively maintained.

@steinm
Copy link
Contributor Author

steinm commented Oct 23, 2025

I have noticed that too, but didn't run into major problems yet. There is a fork at https://github.com/glorieux-f/citeproc-php which has some improvements, but that seems to be abandoned as well. Do you have an example of what didn't work when you used citeproc-php?

@ThoWagen
Copy link
Contributor

ThoWagen commented Oct 23, 2025

For example the style "chicago-author-date" stopped working with v.1.0.2 of https://github.com/citation-style-language/styles

@steinm
Copy link
Contributor Author

steinm commented Oct 23, 2025

That's probably because the page range 'chicago-16' isn't supported yet. I guess that is fixable. I consider to start a new fork of citeproc-php if I find some time.

@EreMaijala
Copy link
Contributor

This is a shot in the dark, but I wonder how much effort would it be to switch to the js implementation instead...

@steinm
Copy link
Contributor Author

steinm commented Oct 24, 2025

Definitely worth looking at. See, if I can find some time.

@steinm
Copy link
Contributor Author

steinm commented Oct 27, 2025

I did some prototyping with citeproc-js. Instead of adding the full citation in cite.phtml there will be just one or more lines like

<div class="citation-csl" data-csl-style="style"></div>

with title and style replaced by the citation style and title being an arbitrary title taken from the configuration in config.ini.
Those lines will be replaced with some javascript by the full citation. I put that javascript into RecordDriver/DefaultRecord/core.phtml though I'm not sure if that is the right place.

  $helper = $this->citation($this->driver);
  $script = "citationdata = JSON.parse('".$helper->getDataCSL()."');\n";

  $this->assetManager()->appendScriptLink('citeproc.js');
  $script .= <<<JS
    VuFind.listen("lightbox.rendered", function listenForLightboxShow(container) {
      VuFind.citeproc.addItem(citationdata[0].id, citationdata[0]);
      modalBody = $(container.container).find('.modal-body');
      citationdiv = $(modalBody).find('.citation-csl').each(function getCitation(i, div) {
        citstyle = $(div).data('csl-style');
        output = VuFind.citeproc.processorOutput(citstyle);
        $(div).html(output);
      });
    });

Actually, we could support both approaches (with and without js) and make that also configurable, e.g. by adding :js to citation_formats[]

citation_formats[] = DIN-1505-2 Citation:din-1505-2:js

There are some minor problems to be solved. In my protoype the style and locale is downloaded straight from github. Those files should be delivered by vufind itself. Which also means, that we still need composer packages for citation-style-language/locales and citation-style-language/styles

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.

4 participants