-
Notifications
You must be signed in to change notification settings - Fork 482
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
[TeX] Allow custom preamble #1788
Conversation
For now, we are ok with the current TOC depth at section level, and don't necessarily want to be able to keep it at chapter level. |
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.
LGTM, just a couple of organizational comments, and a few last things:
- Could use a CHANGELOG entry for this, including a reference to [LaTexWriter] Better spacing between section number and title #1785, since that PR didn't add a CHANGELOG entry.
- It would also be nice to have a note in the documentation about the possibility to override the preamble.
@ViralBShah Let's tag after this is merged? |
|
Linkcheck failed because the new link is only valid when this pr is merged. Temporarily pointing the link to |
Good to merge? |
src/Writers/LaTeXWriter.jl
Outdated
|
||
% Useful variables | ||
\\newcommand{\\DocMainTitle}{$(doc.user.sitename)} | ||
\\newcommand{\\DocVersion}{$(get(ENV, "TRAVIS_TAG", ""))} |
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.
Since we rarely use TRAVIS
anymore, should we just set the \DocVersion
variable to VERSION
?
\\newcommand{\\DocVersion}{$(get(ENV, "TRAVIS_TAG", ""))} | |
\\newcommand{\\DocVersion}{ $(VERSION) } |
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.
Travis is definitely gone. @mortenpi ok?
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 just realized that VERSION
refers to the version of julia, but for users of Documenter.jl, it's more like they want the version of their own package.
Maybe this variable should be deprecated or deleted.
If the user needs the version number to be displayed, just append it to the site name.
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 was just thinking the same thing - I don't recollect what TRAVIS_TAG refers to.
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.
TRAVIS_TAG
: If the current build is for a git tag, this variable is set to the tag’s name, otherwise it is empty (""
).
Environment Variables - Travis CI
And
Documenter.jl/src/deployconfig.jl
Lines 136 to 139 in cd49b07
- `TRAVIS_TAG`: if set, a tagged version deployment is performed instead; the value | |
must be a valid version number (i.e. match `Base.VERSION_REGEX`). The documentation for | |
a package version tag gets deployed to a directory named after the version number in | |
`TRAVIS_TAG` instead. |
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.
So this would just try to determine the package version number automatically, but only works if it's being used on Travis (or if the user sets the environment variable themselves; e.g. it used to work for Documenter). However, I don't think it's actually good that it's quietly relying on the enviroment variable.
I would suggest adding another version
variable to the LaTeX
object. To make this non-breaking and non-behavior-changing, it can still just default to get(ENV, "TRAVIS_TAG", "")
, but I think it should be documented that this default is deprecated. On the Julia side, we can then populate this new option so that the resulting PDFs would actually contain a version number.
Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
We may add `DocVersion` back later
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.
LGTM as is, but are you sure we don't want to make the version number configurable? We could then always pass version = VERSION
in Julia's doc/make.jl
so that at least the Julia PDF always has a version number.
We certainly want a configurable version number. Perhaps we can start by adding a julia version number variable. |
I would just add the version number option to the Documenter.jl/src/Writers/LaTeXWriter.jl Lines 43 to 49 in cd49b07
You can then set it by e.g. In principle, it wouldn't be a bad idea to have it be an argument to |
Yes, maybe open a new issue to track this. This PR is ready for merge. |
Awesome, thanks @inkydragon! Also, I just noticed the uploading of the PDF artifacts --- that's super handy. We should look into managing preview builds with this too. |
Close #1746.
preamble.tex
file to override the default preamble.Ready for review.