-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Update Documenter 0.27.23 => 1.2.1 #47105
Conversation
d64539f
to
b24944a
Compare
This should be good to go, except that #51328 should get merged first. Other than just bringing the various Documenter improvements, one big thing here is that it will fix the source URLs of the standard libraries. |
A couple small fixes and cosmetic improvements to the Unicode table in the docs: * Most importantly, makes sure that the resulting object from the at-eval block is `Markdown.MD`, which which will be necessary for Documenter 1.0 (#47105). It also fixes the `Markdown.Table` structure -- each of the cells should be an array, not just a string, so it adds one more layer of nesting. * Cosmetically, center-aligns the characters, and wraps the latex commands in `<code>`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is green; can we merge?
A few minor comments aside, the diff here LGTM. Unless you think that merging this will cause significant issues missed by CI, let's do this!
doc/make.jl
Outdated
) | ||
end | ||
|
||
@info read(`$(Sys.which("git")) --version`, String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you added a dep on LibGit2
for LibGit2.GITHUB_REGEX
here and Documenter.jl also depends on LibGit2. Is this info statement because Documentor also uses git via shelling out? If not it may be misleading. (somewhat related: JuliaLang/Pkg.jl#2679)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a debug print and I'll remove this. Generally, I need to do a pass on this PR before we merge, to clean it all up.
The issue I ran into here was that Git on the Windows runners is ancient (and I was trying to figure out what version it is exactly). But I resolved that by switching Documenter to Git.jl (which is the better than relying on the system's git
binary anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note here though: kinda silly to depend on LibGit2 just for this regex nowadays. JuliaDocs/Documenter.jl#2363
No, it needs a new Documenter version. It's depending on a branch right now. But soon -- it looks like between fixing the stdlib logic and switching Documenter to Git.jl, it is indeed finally working 🎉 |
Co-authored-by: Lilith Orion Hafner <[email protected]>
Assuming all goes well on the CI, I think this is now ready to be merged. |
Tried this locally and this PR seems to fix JuliaLang/LinearAlgebra.jl#907. |
Oops, sorry I never got around to clicking merge once the CI passed. Thanks @N5N3! |
It looks like this didn't fix the standard library links.. the CI build tree difference is causing yet more issues 😢 (which I can replicate locally if I unpack a tarball into Although, in this case, it also looks like Documenter is doing something a bit fishy, since it's failing to create the source links, but it's also not printing any warnings. |
Currently mainly to test a few things on CI here, but will update this to an actual PR once 0.28.0 is out.