DocumenterCitations 1.3.0 is a breaking change?

[kellertuer]

Just saw on CI that the Pages= argument might have been changed from absolute to relative files? See the error message on CI at

https://github.com/JuliaManifolds/Manopt.jl/actions/runs/6717763433/job/18256227165?pr=294#step:9:1220

┌ Error: Invalid "functions/gradients.md" in Pages attribute of @bibliography block on page src/functions/gradients.md: No such file "src/functions/functions/gradients.md".
└ @ DocumenterCitations ~/.julia/packages/DocumenterCitations/HU6z2/src/expand_bibliography.jl:279

especially the message which file was not found indicated, that removing functions/ should fix it (will try) – which is of course not so much a problem, but a breaking change in a non-breaking update.

[goerz]

No, that’s fixing a bug. The intent was always for it to be consistent.

[goerz]

Although I agree that this update is about as breaking as one might still get away with. I don't think anything that breaking will be in any future 1.x release.

I'm also open to second opinions, and if there's a strong consensus that this is too breaking, I can revert the release and release it as 2.0.

Incidentally, for src/functions/gradients.md: you should now be able to use @__FILE__

[fredrikekre]

Could fall back to the old behavior perhaps?

[goerz]

Not easily. At least, I'd also have to remove the support for @__FILE__, disable a lot of the test suite, and adapt the documentation (which extensively uses the new Pages). And we'd still have to release 2.0, right?

[kellertuer]

Of course this could be considered a bug, if you say that the intended behaviour was to do the same as Documenter does – then I just maybe missed an entry in the changelog about that.

It is also not hard to fix for sure :)

[kellertuer]

Hm, there seem also quite some (probably only breaking in the same way above) breaking changes for the bib file. & has to be escaped – and something with \ as well?

I get

┌ Error: Unsupported command: \textendash
│   supported_commands =
│    34-element Vector{String}:
│     "\\\""
│     ⋮
└ @ DocumenterCitations ~/.julia/packages/DocumenterCitations/HU6z2/src/tex_to_markdown.jl:363
ERROR: LoadError: ArgumentError: Unsupported command: \textendash. Please report a bug.
Stacktrace: ...

and I am a bit lost on

ERROR: LoadError: `makedocs` encountered an error [:bibliography_block] -- terminating build before rendering.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] runner(#unused#::Type{Documenter.Builder.RenderDocument}, doc::Documenter.Document)
   @ Documenter ~/.julia/packages/Documenter/nQAq5/src/builder_pipeline.jl:253
 [3] dispatch(#unused#::Type{Documenter.Builder.DocumentPipeline}, x::Documenter.Document)
   @ Documenter.Selectors ~/.julia/packages/Documenter/nQAq5/src/utilities/Selectors.jl:170
 [4] #79
   @ ~/.julia/packages/Documenter/nQAq5/src/makedocs.jl:248 [inlined]
 [5] withenv(::Documenter.var"#79#81"{Documenter.Document}, ::Pair{String, Nothing}, ::Vararg{Pair{String, Nothing}})
   @ Base ./env.jl:197
 [6] #78
   @ ~/.julia/packages/Documenter/nQAq5/src/makedocs.jl:247 [inlined]
 [7] cd(f::Documenter.var"#78#80"{Documenter.Document}, dir::String)
   @ Base.Filesystem ./file.jl:112
 [8] #makedocs#77
   @ ~/.julia/packages/Documenter/nQAq5/src/makedocs.jl:247 [inlined]
 [9] top-level scope
   @ ~/Repositories/Julia/Manopt.jl/docs/make.jl:133

[goerz]

The "intended" behavior wasn't documented, which makes this a tricky case. But the old behavior definitely wasn't intentional.

I was on the fence on releasing this as 2.0 anyway, even more so because the "internals" broke so significantly, and we're only getting away with that because we've been defining the "SemVer-surface" of this package to be extremely small. This is also why I collected a large bunch of semi-breaking PRs into this 1.3 release.

To be honest, I feel like tagging a 1.0 release when the project moved to the JuliaDocs was a bit pre-mature since I wasn't quite sure that everything was fully worked out yet. We're getting to that point, though: The 1.3 release fixes all major issues, in my mind. I'd consider it a "release candidate" for declaring all of the documented "internal" functions "stable", which probably would have been a better 1.0. Alas.

So the choice is more or less between accepting that 1.3 is somewhat breaking (non-breaking only with some mental gymnastics), with an understanding that things are on a path to stabilizing. Or, we tag is as 2.0, and embrace the "Breaking releases aren't a big deal" idea that some SemVer proponents have advocated for. It might be for the best: I'd expect to stay on 2.x for a good long while even in that case, but if something "breaking" ever comes up again in the future, the mental barrier for going to 3.x would be much smaller.

[kellertuer]

And that is also fine with me then - I just still have to fix my library now 🧐

[goerz]

& has to be escaped – and something with \ as well

Yes, I think so… the same escaping rules as in LaTeX. In the previous version, an unescaped & would work with DocumenterCitations, but it wouldn't have been valid tex (that is, if you use the same .bib file with latex/bibtex, it should have errored). If there's a discrepancy between LaTeX escaped characters and DocumenterCitations, that would be a (new) bug.

The details of how exactly the strings in the .bib files are processed may not ever be fully "stable". The general guiding principle is for it to emulate LaTeX as closely as possible, at least for some supported subset of LaTeX. So yes, changes here should be considered in the "bugfix" category (even if they "break" an existing .bib file). Although again: things should stabilize over time.

Unsupported command: \textendash

I have never seen \textendash before. I can easily add support for it, but shouldn't that just be -- in the .bib file? In the previous version of DocumenterCitations, that \textendash would probably just have shown up in the rendered output, which probably isn't what you wanted ;-)

In general, I've only added support for a tiny subset of tex commands outside of math mode (really just \url, \href, \textit, \texttt, I think), with the assumption that people will let me know if they have other commands that occur "in the wild". They're very easy to add, and any new command would be considered a "bugfix".

So, let me know if you want support for \textendash (or any other command), or if you'd rather rewrite that as the standard --.

ERROR: LoadError: makedocs encountered an error

Is that in the same run as the Error: Unsupported command: \textendash?

If so, that's the problem. If ERROR: LoadError: makedocs encountered an error is the only error message, that would be something for me to look into.

I agree it's not great that Documenter shows a Stacktrace when it fails in strict mode. It makes it look like an internal error. That's what you're concerned about, right?
Unfortunately, I think that's just how it is in Julia. It would be great if there was an error(…; stacktrace=false) option. In any case, that's probably more something to discuss within Documenter, although I'm also open to any creative ideas on how to improve the UX here.

[kellertuer]

Indeed I had an \textendash in my bible, no clue why. Maybe I was already a bit tired from fixing a few things (also in my PR not only here) – the second error is already a follow up; I went home by now, but also pushed to the PR, see

https://github.com/JuliaManifolds/Manopt.jl/actions/runs/6722564479/job/18270768694?pr=294#step:9:1320

Maybe before leaving I missed that a few more pages need to be adapted tomorrow (most seem to be the pages issue above) – it is just a bit surprising that Bibliography is just issuing warnings but Documenter fails on that ;)

[kellertuer]

I think I am slowly getting there. I am a bit surprised setting DOI = to a valid URL leads to

┌ Warning: Could not link ["https://doi.org/10.1017/9781009166164"] in "published in" information for entry Boumal:2023. Add a Note field that links to the URL(s).

edit: Thaht is my only entry with a DOI and an URL to something different. Will add the url for now as a note. It would be great if the URL can be different from the DOI ;)

[goerz]

I am a bit surprised setting DOI = to a valid URL leads to [Error]

That's always been the case… The DOI field should contain a DOI (starting with 10.), not a URL ;-)

It's a common mistake, though. Maybe I'll detect it in some future version, and correct it automatically.

Maybe you didn't get an error before because we "accidentally" fixed #58 in this release. But you would have had broken links before. Moreover: putting a URL in the DOI field shouldn't work in LaTeX, either.

It would be great if the URL can be different from the DOI

Absolutely, and this has been supported since 1.0. There's quite a few examples in the bibliography for entries that have both a URL and a DOI. For articles, the URL always gets linked via the title, and the DOI always via the "published in" information. For non-articles, things are a bit less regular: Either the DOI or the URL get linked via the title, and if there are more URLs to link, they'll be linked via the booktitle. If there's both a DOI and a URL but not both a title and a booktitle, you'll get an error. In that case, the correct thing to do is to put the URL in the note field (preferably with \url/\href).

[kellertuer]

Sorry I was not precise enough, I used a valid DOI but an URL pointing to a book page, that lead to a warning and that warning makes documenter fail.

[kellertuer]

...or to be precise this

https://github.com/JuliaManifolds/Manopt.jl/blob/f36115fd34c7b2f5345bc5b22681e3c58227bd03/docs/src/references.bib#L198-L209

should work but putting the URL from the note into an URL= field does not work and conflicts with DOI. I hope for now my ops work again (yay!) and I can write an issue about this more precisely later.

[goerz]

Ah, yes, I think for a book you're running exactly into this situation where there is only one title field to link from (no separate title and booktitle). In the previous version of DocumenterCitations, this might have worked, since I was linking the URL to the title and the DOI from e.g. the publisher. That didn't really work consistently across all the different types of entries. So, in order not to lose my mind, I made it a bit more restrictive. And yes, putting the URL in a note like you have right now is the correct approach for this (I think you're missing \href there, but that's just a detail)

[kellertuer]

Ah, another breaking change! Formerly the BibTeX fields Markdown rendered, that is why I used a Markdown link [text}(url) but then now they need to be latex?

edit: Checked it real quick – as soon as one does not. confuse again the German and the Norwegian Keyboard and uses the right brackets and the right order – Markdown links work just fine sill in those fields.

[goerz]

Yes, the .bib file should be standard latex now (as one would expect). I think before, it was actually allowed to be html (despite me saying at some point that it was markdown). But that was very much relying on undocumented internals (and I think I was pretty upfront at the time that wasn’t a “feature” to rely on in the long term).

But yeah, this release is about as full of breaking internals as it gets, which is kinda why I collected it all in one pretty big release. Hopefully, you won’t have to go through any of this again until an actual 2.0 release.

[goerz]

Wait, markdown links still work? I’m not sure they should… or if they do, it’s definitely exploiting internal behavior. Going forward, having markdown in the .bib file is definitely not supported / recommended. Instead, I’ll add support for any reasonable latex commands anyone might request.

[goerz]

The markdown links indeed still work, but be warned: that's absolutely exploiting a bug (#60)

I would recommend using the eprint field for preprint information.

[goerz]

#59 (comment):

Could we fall back to the old behavior perhaps?

Actually, maybe we can: for any name in Pages that can't be found as a file relative to the folder which contains the file containing the @bibliography block, we could check if it exists relative to the src dir (which was the effective pre-1.3 behavior). I yes, we can @warn that Pages should be updated. That should be relatively safe, and would probably ease the transition a lot.

Let me think about that a bit more…

[kellertuer]

So to summarise – no it is not breaking in the sense that the changes are bugfixes, since the inention was from the beginning to be closer to Documenter.jl anyways.

For future releases with bug fixes one could write in the change log something like: That was the old version that is wrong, the better way to write it would be this.

[goerz]

That's right. The release notes for 1.3 should have had better "upgrade instructions". I've now added these in NEWS.md (scroll down to the bottom of the 1.3 section).

#61 also adds a fallback so that the pre-1.3 Pages doesn't break quite as badly.

[goerz]

#59 (comment)

┌ Error: Unsupported command: \textendash
│   supported_commands =
│    34-element Vector{String}:
│     "\\\""
│     ⋮
└ @ DocumenterCitations ~/.julia/packages/DocumenterCitations/HU6z2/src/tex_to_markdown.jl:363
ERROR: LoadError: ArgumentError: Unsupported command: \textendash. Please report a bug.

Is that unabridged? Because I wanted the supported commands to actually be printed in that message (not 34-element Vector{String}). That would be something for me to fix in 1.3.1.

[goerz]

#59 (comment):

For non-articles, things are a bit less regular: Either the DOI or the URL get linked via the title, and if there are more URLs to link, they'll be linked via the booktitle. If there's both a DOI and a URL but not both a title and a booktitle, you'll get an error

#59 (comment)

for a book you're running exactly into this situation where there is only one title field to link from (no separate title and booktitle). In the previous version of DocumenterCitations, this might have worked, since I was linking the URL to the title and the DOI from e.g. the publisher.

Not only it "might have worked", but it did work. So, after some reflection, I kinda agree that this wasn't a great move (and arguably a semver violation). As a bugfix, I'll restore the ability to always have both a DOI and a URL field automatically linked for any valid BibTeX entry, see #63.