-
-
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
Manual Conversion #18588
Manual Conversion #18588
Conversation
@@ -29,7 +29,7 @@ end | |||
""" | |||
@enum EnumName EnumValue1[=x] EnumValue2[=y] | |||
|
|||
Create an [`Enum`](:obj:`Enum`) type with name `EnumName` and enum member values of | |||
Create an `Enum` type with name `EnumName` and enum member values of |
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.
is there going to be autodetection of some kind for references like these?
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.
Enum
itself doesn't actually have a docstring, only @enum
does. For some reason sphinx didn't ever complain about that.
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.
Re: autodetection, I think we can probably add, at some point, a check to Documenter that will suggest non-references that could be references.
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.
Ah. Was this the case for all of the ones you removed in this commit?
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.
Yes, there's a whole bunch that aren't links any more: Integer
, Real
, Rational
, etc.
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.
And none of those types have docstrings?
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.
Similarly for UInt, Int, Float16 types. I think seems like this could be fixed in a later PR. Linking every type_name
automatically seems like a neat Documenter feature!
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.
Those ones as well. What I'll do is put together a list of which ones are missing after this conversion is done and open an up-for-grabs issue to track which ones still need documenting.
I'm hoping 43f05c2 was automated. Is there a script you have that did that? Would it be possible to separate that huge commit into one commit per modified file, so there's a way to get github to show the whole diff (one piece at a time)? |
Yes, pretty much all automated. I should be able to do one commit each I think. (Seems like that worked...) |
ace2fab
to
a7579dd
Compare
I'm inclined to keep the paragraphs in the |
It can be a bit annoying to look at doc diffs of super long soft-wrapped lines, since you get the whole thing before then the whole thing after, and it's tough to spot specific changes. Github does a better job highlighting short-line diffs than long-line diffs from what I've seen, I personally think a diff with re-wrapping the start and end of a bunch of lines is easier to check than a diff where you can't tell what actually changed. |
I can see if hard-wrapping them looks better, will be later this evening though. |
end | ||
|
||
and not a ``x`` in the scope where ``foo`` is used:: | ||
# [Scope of Variables](@id scope-of-variables) |
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 checked this file and no text has been lost, all links work. There are some minor formatting issues, which I will report when requested. I noted that there is no highlighting in julcon
blocks.
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.
jlcon
is being used for all code blocks containing Julia REPLs since plain julia
ends up highlighting results incorrectly sometimes, such as
julia> x -> x
(::#1) (generic function with 1 method)
jlcon
is used in the pygments highlighter to correctly highlighting Julia REPL blocks, so there's precedence for that naming convention. We're using highlight.js for highlighting, which I hope we can add an equivalent jlcon
highlighter to eventually.
What were the other formatting issues you encountered?
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.
The section uses mixed REPL non-REPL blocks (which may not be clever although readable), see e.g. https://michaelhatherly.github.io/julia-docs/manual/variables-and-scoping.html#Local-Scope-1, those used to be highlighted better.
The table right at the start was nicer before with sub-tables but no big deal.
https://michaelhatherly.github.io/julia-docs/manual/variables-and-scoping.html#Hard-Local-Scope-1: the second paragraph starting with "In a hard local scope, ..." was some kind of block & indented. The purpose was to make it stand out more.
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.
The section uses mixed REPL non-REPL blocks (which may not be clever although readable)
Yes, mixed blocks seems, to me, like something we should avoid for the sake of consistency. (Note, we could just highlight all blocks as Julia code and get back to the same highlighting as before I think.)
The table right at the start was nicer before with sub-tables but no big deal.
Bit unfortunate, but markdown doesn't have a standard syntax for sub-tables, so that was the best approximation I could come up with. I would like to eventually add sub-tables, probably based off the pandoc ones.
the second paragraph starting with "In a hard local scope, ..." was some kind of block & indented. The purpose was to make it stand out more.
Seems the blockquote wasn't converted correctly.
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.
And, yes, maybe use Julia-highlighting for now.
a7579dd
to
3a3c77d
Compare
Hard-wrapped paragraphs, fixed the blockquote issue @mauro3 found, and now using |
0fbc4c3
to
54447f5
Compare
c48d043
to
48731a5
Compare
@#Check documentation | ||
@$(JULIA_EXECUTABLE) $(JULIAHOME)/doc/NEWS-update.jl #Add missing cross-references to NEWS.md | ||
@$(MAKE) -C $(BUILDROOT)/doc unicode #Rebuild Unicode table if necessary | ||
@$(JULIA_EXECUTABLE) $(JULIAHOME)/doc/DocCheck.jl > $(BUILDROOT)/doc/UNDOCUMENTED.rst 2>&1 #Check for undocumented items |
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.
should we port this or is this totally bitrotted and useless right now?
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.
Documenter provides pretty much the same features for checking for missing docs. Should be fine to remove.
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 think this never actually did anything since it's just defining a module but not running any of the code it defines
@$(MAKE) -C $(BUILDROOT)/doc html SPHINXOPTS="-n" #Rebuild Julia HTML docs pedantically | ||
@$(MAKE) -C $(BUILDROOT)/doc latex SPHINXOPTS="-n" #Rebuild Julia PDF docs pedantically | ||
@$(MAKE) -C $(BUILDROOT)/doc doctest #Run Julia doctests | ||
@$(MAKE) -C $(BUILDROOT)/doc linkcheck #Check all links |
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.
pdf, doctest, and linkcheck all need to run in release-candidate. In strict mode if Documenter is going to differentiate.
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.
Currently there's no PDF output option for Documenter, so if that's deemed important enough to have working as soon as we switch over then this PR will need to wait until I've got some time to implement and properly test it first.
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.
0.6rc is kinda a long way off, surely that doesn't need to hold up this PR.
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.
The pdf is pretty widely used as I understand it, but not necessarily from master. It's okay for it to be missing from master for a few weeks, but any longer than that would not be okay as the plan is actually to do 0.6 on a very abbreviated schedule if we can.
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.
We cannot gate the transition on being 100% feature complete. If someone wants the PDF badly enough, they can implement that feature in Documenter.
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.
Okay, but we can't afford to be in a state that we can't release from for very long. Not providing a pdf is a pretty big regression, the pdf form of the manual is the way many people start learning Julia.
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.
What I'll do is see whether I can get a latex/pdf output sorted during the next week. It's definitely not going to need as much work as the html, so it might very well be doable. (Hopefully we aren't wanting any other formats, are we?)
At the moment this branch is really trivial to rebase over any changes, so I'm not too worried about rushing it in if there's things that can be fixed beforehand that would make the transition better.
(Out of interest does readthedocs provide any stats on number of downloads of the pdf vs number of website hits?)
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.
See also: JuliaDocs/Documenter.jl#4. If anyone has any particular requests for how they'd like the pdf docs to be styled/structured then comments over there are very welcome. I'll probably try to follow the current sphinx style reasonably closely where possible.
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.
For those not following, @MichaelHatherly added pdf support to Documenter: https://dl.dropboxusercontent.com/u/14501513/julia.pdf
It seems like the key question remaining here is:
decide how to deploy the generated docs;
|
||
@# Check to see if the above make invocations changed anything important | ||
@if [ -n "$$(git status --porcelain)" ]; then \ | ||
echo "Git repository dirty; Verify and commit changes to the repository, then retry"; \ |
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.
put this back
|
||
deps: | ||
@echo "Installing documentation dependencies." | ||
$(JULIA_EXECUTABLE) -e 'Pkg.installed("Documenter") === nothing && Pkg.add("Documenter")' |
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.
don't modify global package state - better to put a REQUIRE file in here and set JULIA_PKGDIR while running the commands for this
(Apologies for the delays in finishing this up, was travelling. I'll try finish it off over the weekend.) |
48731a5
to
840d18a
Compare
Rebased, some makefile changes for doctest/linkcheck targets, and install deps in a separate The generated docs available here: |
unicode: $(BUILDDIR)/manual/unicode-input-table.rst | ||
doctest: | ||
@echo "Running all embedded 'doctests'." | ||
$(JULIA_EXECUTABLE) make.jl -- doctest |
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.
is there a strict mode yet for checking releases?
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.
Not just yet, I'll add one shortly.
looks like paragraph division got lost here https://michaelhatherly.github.io/julia-docs/devdocs/reflection.html |
Thanks, those are |
840d18a
to
662f5b8
Compare
Are there notable differences between the Windows buildbots and Appveyor? Output redirection has worked pretty well there for a while. I guess we could just disable doctests when building there, or perhaps just only build on the Linux ones since the docs should really be identical no matter where they were built and reuse those elsewhere.
Not sure what to make of that error,
since I believe I used the command path as was already in use. Is there a better way to get the path to the |
Appveyor doesn't run |
Are there any workarounds for #11727? I could possibly hack something into Documenter to just disable anything that uses any redirection, bit of a heavy handed solution though. |
Redirecting to a file might work, or suppressing the doc output when run under |
Not sure what changed, but that path wasn't correct in general. See #19539. Though even without that change, there's another problem to build RPMs: Internet access isn't available on build machines. So I'll need to find a way to download the packages beforehand. We should perform this step automatically from (Note that these complaints are minor compared to the great improvement made by this PR.) |
``:push_loc``: enters a sequence of statements from a specified source location. | ||
- ``args[2]`` specifies a filename, as a symbol. | ||
- ``args[3]`` optionally specifies the name of an (inlined) function that originally contained the code. | ||
* `:push_loc`: enters a sequence of statements from a specified source location. |
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 didn't previously have a bullet, though the inline
and pop_loc
lines probably should instead of the way it was
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.
Yes, that page is a bit messy due to use of rst definition lists which our markdown doesn't really have an equivalent for. I'll try tidy things up a bit there.
I suppose that kind of ties into whatever we decide to do with #18795. |
Actually, I would be both more logical and easier to build the docs during the |
Should the doc-links on julialang.org be udated? Or are the new docs still "secret"? |
Yes, links need to be updated. Has to be done by someone with access to the domain. @StefanKarpinski would you be able to sort that out when you've got a spare moment? Thanks.
Is it actually necessary any more to build the docs on any of the buildbots? When we were using Sphinx it did make sense since readthedocs built slightly different docs to a local sphinx build. Documenter's builds are all exactly the same wherever they are built, so we could potentially just download the files from |
We could, but then people wouldn't be able to generate custom tarballs after modifying the source. Not a big deal, but I prefer generating documentation on the fly to match exactly the tarball which is generated than rely on an external documentation source. Anyway, the fix was pretty simple, I've added it to #19539. |
Would they not then just run |
Great work! Do you consider to provide the documentation also in .epub format? It's been published in epub at least since Julia 0.2, that's how I learned the language which I found very handy at the time. |
Thanks. epub is reasonably similar to html so it's probably not going to be too much work to add a new backend to Documenter to support it, but unless someone else implements it it'll have to wait a while until I've got some spare time to do it. @Ken-B can you open an issue over on Documenter's tracker so this doesn't get forgotten? |
|:------- |:--------- |:-------------------------------- | | ||
| Native | `julia_` | Speed via specialized signatures | | ||
| JL Call | `jlcall_` | Wrapper for generic calls | | ||
| JL Call | `jl_` | Builtins | |
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.
does markdown not have a way of making table entries extend across multiple rows?
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.
Nope, markdown doesn't usually have that I think, though there are some dialects, such as pandoc, that do seem to support those kind of things, so we could probably just extend our parser as well.
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.
Cool. In case you hadn't guessed, I've been checking a few pages using http://services.w3.org/htmldiff, looking at a branch on my fork where I pushed the last pre-conversion commit's html docs (https://github.com/tkelman/julia/commits/gh-pages-preconversion). Haven't done too many yet, but the ones I've looked at have been remarkably clean.
We do need to figure something out for the windows buildbots though.
with documentation build, ref #18588 (comment) redirecting to a file should work, but redirecting to a cygwin pipe or driving application like perl or python (in the buildbot case) doesn't
* Work around redirection issue on windows buildbots with documentation build, ref #18588 (comment) redirecting to a file should work, but redirecting to a cygwin pipe or driving application like perl or python (in the buildbot case) doesn't * Simpler case that's not as windows specific * Revert "Simpler case that's not as windows specific" This reverts commit 4424c54. Unfortunately this doesn't stop on possible failures in the doc build
|
||
function unicode_data() | ||
file = normpath(JULIA_HOME, "..", "..", "doc", "UnicodeData.txt") | ||
url = "http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt" |
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.
Downloading this list from an external source doesn't sound like a good idea to me. That means the data can change without Julia supporting newly-introduced forms (since IIUC this depends on the utf8proc version we use). How about adding this file to git? Or couldn't we get this information from utf8proc itself?
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.
Yes, probably would be better to use a stable source for it. With this rewrite I just made sure it was doing the same thing as before rather than going and changing anything like that.
Or couldn't we get this information from utf8proc itself?
The file is only used to lookup the unicode names for chars, so if utf8proc can do that then that would be great.
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.
Downloading this file also seems to cause build failures, e.g. https://travis-ci.org/JuliaLang/julia/builds/186607980 ? Best!
Edit: Seems that download attempt has been causing build failures regularly since last night?
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.
Hm, guess we should just commit that file then (though it's a bit big perhaps), or maybe host it ourselves somewhere more reliable.
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.
@stevengj Your opinion on this?
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.
utf8proc doesn't include names for unicode characters, so it won't help with this.
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.
we shouldn't commit it, but we should be specific about what version we download (not latest) and use juliacache where this is probably already mirrored
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.
That would certainly improve the reproducibility of builds. I guess we should also include that file in the tarballs? It would be weird not to be able to build Julia and its documentation offline from the full tarball with dependencies, just because of that missing file.
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.
We put the result of the doc build in the tarball, I think it's okay to not include all the inputs if we include the output. Re-generating it isn't terribly hard with a few packages and additional files.
This moves the manual to markdown and uses Documenter.jl to generated the HTML docs.
TODO items:
doc/make.jl
into theMakefile
s;doc/
directory;I've opened the branch for review now since it is at the point where further changes will need to be done manually and will become difficult to rebase (these changes touch most of
doc/
andbase/
) easily.My suggestion would be to check that the converted docs haven't lost any information, all links still point to the correct places, and address the other TODO items above. After that has all been verified then merge this branch and further polishing and rearranging and/or restructuring of the manual can take place with less chance of running into merge conflicts.
Currently the "doc checks" part of the Documenter build is disabled. This includes checks such as doctests, missing docs, and missing/duplicate footnote checks. Fixing the warnings currently being raised when enabling these checks will each need individual attention, but can be split over several PRs to make reviewing easier. In my mind these checks are only absolutely necessary for a released version of Julia, and for
master
we can probably manage without them for a couple of days. If anyone feels that this isn't a good trade-off then I'll fix everything in this PR, but it may take a while longer to get it done properly.Here's a link the to generated docs to make reviewing a bit easier: https://michaelhatherly.github.io/julia-docs/. Note that source links for docstrings will 404 since the commits they point to aren't on
master
.