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

Render citations links as arbitrary markdown AST #43

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

goerz
Copy link
Member

@goerz goerz commented Oct 9, 2023

This is a substantial rewrite of the internals handling the rendering of citation links. Before, a citation link ([key](@cite), and all its variations) would stay a link, just with a different link text (as determined by format_citation for a particular style) and a different link target (an item in a bibliography block). Now, format_citation gets a CitationLink object (a parsed version of the original link) and returns a string with arbitrary markdown to replace the entire original link (not just the link text). Typically, the returned markdown will be something like "[[1](@cite GoerzQ20222)]". That is, the replacement will contain another "direct" citation link, which is then rendered automatically with a single level of recursion.

The enhanced flexibility enables several new features and fixes existing bugs:

  • We can have multiple comma-separated citations in a single cite link. For the default :numeric style, these can be compressed ("Refs. [1–3]").
  • We can handle missing references (in non-strict mode), showing question marks, just like LaTeX does it
  • "Citation comments", like "Eq. (1)" in [GoerzQ2022; Eq. (1)](@cite) can now contain inline markdown (math, code, emphasis, etc.) that is preserved in the output
  • We now support citation keys with underscores or anything else where the markdown parser was getting in the way.

The added complexity of the new approach was starting to turn the source code files a bit lengthy, so I've reorganized the code a little, collecting the implementation for the different built-in styles in the styles subfolder. This also allowed to remove some code duplication between the :numeric and :alpha styles. Internal functions that implement format_citation and format_bibliography_reference for the different builtin styles are now exposed in the documentation and simplify the implementation of custom styles. See NEWS.md for more details about the change of the internals.

Closes #6
Closes #14
Closes #34
Closes #41

@goerz
Copy link
Member Author

goerz commented Oct 9, 2023

It might be worth revisiting what we define as the stable API of the DocumenterCitations package. If we take the narrow definition where everything mentioned on the "Internals" page is up for grabs, then the above changes are non-breaking. Specifically they are not breaking for anyone using DocumenterCitations using only the built-in styles.

However, the PR is very breaking for anyone who was using custom styles, including the official example custom styles. I'd be open to an (even informal) policy of making a major version release if we break the internal routines that are documented for use with custom styles, as a courtesy. In general, I'd only change these routines if absolutely necessary, and the only other issue where I would foresee having to break compatibility again is #18.

I'd go with a majority opinion here, especially from those who have mentioned that they may be using custom styles (@lgoettgens, @kellertuer, @nathanaelbosch).

With the narrowest definition of the public API (which is the official one at the moment), the public API is really just the DocumenterCitations object itself and the syntax for the citation links and bibliography blocks: I don't see that we would ever break that, so this package would be a perpetual version 1.* (which is also fine by me).

@kellertuer
Copy link
Contributor

kellertuer commented Oct 9, 2023

I did not use a user defined style – and I see the two sides you are arguing – but maybe you are also “tricking” a bit with only exporting the object and hence declaring that the public API?

Maybe a way in between would be fair. With the current public API this PR is non-breaking.
You could still add a note to the news that it does break the internal structure that might have been used by some
You could further make the parts that users should use for their own styles part of the public API? That way users could more confidently use these parts of DocumenterCitations (and the next release that changes those would be breaking).

edit: besides that the changes sound great! Looking forward to trying them :)

@goerz
Copy link
Member Author

goerz commented Oct 9, 2023

Preview is at https://juliadocs.org/DocumenterCitations.jl/previews/PR43/

See especially:

@goerz
Copy link
Member Author

goerz commented Oct 9, 2023

Maybe you are also “tricking” a bit with only exporting the object and hence declaring that the public API

Yeah, I'd definitely be okay with releasing this as 2.0. and to label the "Internals" page as "this is stuff you only need to know if you want to contribute to the development of the package, or if you want to implement custom styles. It's not exported, but it's still part of the stable API".

The main counterargument would probably just be the general aversion against breaking releases. We only tagged 1.0 a couple of months ago, so it might be "too soon".

If we do decide to tag this at 2.0, then I'd like to think about the best way to implement #18 (which came up on Slack today) before making that release. There's different ways to approach #18, but at least one way would probably also involve a breaking change to the documented internals (the various format_* routine would have to know which Writer is rendering the documentation; more on that tomorrow). It's the only other breaking change of the internals that I can see on the horizon, so I think we might be pretty stable from 2.0 on out.

@kellertuer
Copy link
Contributor

Oh, I was actually advocating to not release it as 2.0 ;)

But if you label this 1.3 – and make the parts that users should use/modify public, a next change like this would be (API-)breaking which gives users more confident in using things like the custom styling.

@lgoettgens
Copy link
Contributor

I am very much in favor of declaring everything that is suggested in the docs to be used for custom styles as part of the API, i.e. this change would be considered breaking.

For my own projects with custom styles, however, this would be fine, if this PR gets released in the near future, as I didn't find the time to finally merge everything over there, yet. So I, personally, would be fine with the procedure described by @kellertuer.

@goerz
Copy link
Member Author

goerz commented Oct 11, 2023

Ok, that seems to be the consensus, then. We'll release this is at 1.3, but then declare the documented internal API as "stable" at some point in the near future – when I've made sure everything is in a state that I'm happy with for the foreseeable future. I'll make that official switch in its own release, so probably 1.4.

If there are no further comments, I'll merge this PR on Friday, and then work on #18, which will also make it into 1.3 (and is also slightly breaking)

Change `format_citation` so that is returns markdown code that replaces
the entire original link.

* Enable multiple citation in a single cite link, which can be collapsed
  in the default `:numeric` style
* Support citation keys with underscores
* Handle in-line formatting in "citation notes"
* Handle missing references in non-strict mode by printing question
  marks (just like LaTeX)

Substantial restructuring of internals
@goerz goerz merged commit 7b9e4dd into master Oct 13, 2023
15 checks passed
@goerz goerz deleted the mg/collapsed-citations branch October 13, 2023 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants