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

Deploy docs with clean URLs #22048

Merged
merged 4 commits into from
Jun 3, 2017
Merged

Deploy docs with clean URLs #22048

merged 4 commits into from
Jun 3, 2017

Conversation

mortenpi
Copy link
Contributor

@mortenpi mortenpi commented May 24, 2017

Switch the deployed docs to cleaner URLs like manual/linalg/ instead of manual/linalg.html. Requires Documenter v0.11.0.

The new Documenter version brings a couple of other updates as well, most importantly a decent mobile experience (many thanks to @wookay for that). I hope I'm not too late to the party, but it would be great to have this for the 0.6 release docs as well.

There are a couple of loose ends:

  • Local builds should still go with the page.md -> page.html convention, i.e. html_prettyurls should be false for them. I made the option depend on the deploy argument, and I propose two different targets for the docs Makefile -- html for local builds and a separate deploy just for deploying from Travis. But it also requires support from the main Makefile, and I could use some advice there.
  • One of the links in the manual is broken due to a bug in Documenter (ref to "Unicode Input" in manual/interacting-with-julia.md).
  • Updated Documenter also complains about links that start with a #, but that doesn't actually break any URLs in the output.
  • Broken search links.

An example build of the manual: http://mortenpi.eu/julia/pretty-urls/

@mortenpi mortenpi mentioned this pull request May 24, 2017
@kshyatt kshyatt added the docsystem The documentation building system label May 25, 2017
@kshyatt kshyatt requested a review from MichaelHatherly May 25, 2017 01:00
@tkelman
Copy link
Contributor

tkelman commented May 25, 2017

would be good to diff the generated html before vs after this. I'm a bit nervous that the refactorings here might change some of the windows backslash fixes I had to make in Documenter.

@mortenpi
Copy link
Contributor Author

Good point, but it seems to look fine. I kept the replacements intact and all the path math should go through the relhref function anyway, where the replacement is definitely being done.

Windows builds of the docs are available here: https://github.com/mortenpi/julia/tree/gh-pages

I also have a tidyed diff for search.html/search/index.html with v0.10.3 and v0.11.0, respectively: https://gist.github.com/mortenpi/f328e2d2419647384f8b7d5d77b64f79

And when grepping for double backslashes I didn't see any that were in links.

I built Julia under Cygwin for this, and I also checked that the broken URLs were indeed present when building the docs with Documenter v0.9.0, which didn't have your fixes yet.

doc/Makefile Outdated
@cat docbuild.log
endif
@echo "Build finished. The HTML pages are in _build/html."

deploy: html
deploy: ARGS := deploy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a pointer to make docs on what this is doing? not a pattern I see often

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sets the ARGS variable, but only for the deploy target, and it gets passed on to all the dependencies, hence I can use it in html. https://www.gnu.org/software/make/manual/make.html#Target_002dspecific

But I have no idea if this is a good pattern to use or not, just the only thing that came to mind.

Copy link
Contributor Author

@mortenpi mortenpi May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. the only thing which wouldn't involve copying the whole html target.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so what will ARGS be when running the html target? empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. The command that gets executed in my case is

/home/morten/Julia/julia/build/usr/bin/julia /home/morten/Julia/julia/doc/make.jl --

It might be a good idea to explicitly set it in the Makefile though, to make that clear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makefiles are confusing enough that unless we eventually start using a system that is actually auto-generating them, readable is probably preferable to overly-clever for the sake of brevity

@mortenpi
Copy link
Contributor Author

I got rid of the variable magic. I didn't copy the Windows-specific part for the deploy target since it's not really relevant on Travis. Any thoughts on how to integrate this into .travis.yml or the main Makefile though? A make -C doc/ deploy somewhere?

@jebej
Copy link
Contributor

jebej commented May 27, 2017

Currently, clicking on search results does not work: JuliaDocs/Documenter.jl#495

@mortenpi
Copy link
Contributor Author

mortenpi commented May 28, 2017

Various path issues fixed by v0.11.1. The "Unicode Input" link issue was due to the page being listed twice in pages (ideally Documenter would detect this situation; JuliaDocs/Documenter.jl#497). Example build updated: http://mortenpi.eu/julia/pretty-urls/

Could still use some advice on how to incorporate this into .travis.yml so that the docs would deploy automatically.

doc/REQUIRE Outdated
@@ -1,3 +1,3 @@
Compat 0.25.0 0.25.0+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also bump this to 0.25.2 0.25.2+ to get rid of some deprecation warnings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mortenpi mortenpi changed the title WIP: Deploy docs with clean URLs Deploy docs with clean URLs Jun 1, 2017
@mortenpi
Copy link
Contributor Author

mortenpi commented Jun 1, 2017

So, putting the deploy step into after_success feels reasonable. It seems to run OK.

@tkelman
Copy link
Contributor

tkelman commented Jun 1, 2017

looks like there are a few invalid links?

369.426951  !! Invalid local link: unresolved path
369.427061     'linalg.html#stdlib-blas-side-1' in stdlib/linalg.md
369.427100  !! Invalid local link: unresolved path
369.427120     'linalg.html#stdlib-blas-uplo-1' in stdlib/linalg.md

 !! Invalid local link: unresolved path
    '../numbers/#Core.Float64' in stdlib/arrays.md

@mortenpi
Copy link
Contributor Author

mortenpi commented Jun 1, 2017

Yup -- I think they are from commits that came after the one this PR is forked from. I'll rebase before merge and fix them, but is there anything else besides this?

@tkelman
Copy link
Contributor

tkelman commented Jun 1, 2017

don't think so. I'd leave off the trailing / in make -C doc/ but that's pretty inconsequential

mortenpi added 2 commits June 1, 2017 20:59
The "Interacting with Julia" page was listed twice in the sidebar.
mortenpi added 2 commits June 1, 2017 21:17
Documenter has to be bumped to v0.11.1 for this. Also change two
absolute URLs that will become outdated with this change to the more
appropriate relative URLs.

Also bump Compat to get rid of deprecation warnings.
There is only one 5-argument docstring:

  symm(side, ul, alpha, A, B)
@mortenpi
Copy link
Contributor Author

mortenpi commented Jun 1, 2017

It turns out that those "invalid links" were Documenter errors after all. In short -- if the same docstring gets included multiple times then Documenter currently calls fixlinks! multiple times on it, and complains on the second time since the link is already fixed. But it shouldn't break anything in the output, so I'd leave fixing that for another day (JuliaDocs/Documenter.jl#505).

A separate issue is why we have multiple copies of a docstring in the first place. The ../numbers/#Core.Float64 one comes from speye, where Base.SparseArrays.speye(::SparseMatrixCSC) in the manual also includes the docstring for Base.SparseArrays.speye(::Type, ::Integer, ::Integer), which looks like a bug in Documenter to me.

The linalg.html ones come from the symm function, which had both Base.LinAlg.BLAS.symm(::Char, ::Char, ::Any, ::Any, ::Any) and Base.LinAlg.BLAS.symm(::Any, ::Any, ::Any, ::Any, ::Any) listed. The one with Chars doesn't have a docstring actually, so I removed the reference for that. Whether the one with ::Chars should match the one with only ::Anys in the first place, I am not actually sure -- I don't think the type signature matching logic is clearly documented in Documenter at the moment.

In any case I would leave these for another day as well (JuliaDocs/Documenter.jl#506).

Removed the / -- consistent with the rest this way.

@tkelman tkelman merged commit 9a22471 into JuliaLang:master Jun 3, 2017
tkelman pushed a commit that referenced this pull request Jun 3, 2017
* Move deps .PHONY together with others

* Remove duplicate page from manual

The "Interacting with Julia" page was listed twice in the sidebar.

* Build docs with clean URLs

Documenter has to be bumped to v0.11.1 for this. Also change two
absolute URLs that will become outdated with this change to the more
appropriate relative URLs.

Also bump Compat to get rid of deprecation warnings.

* Remove duplicate symm docstring reference

There is only one 5-argument docstring:

  symm(side, ul, alpha, A, B)

(cherry picked from commit 9a22471)
@mortenpi mortenpi deleted the clean-urls branch June 3, 2017 03:07
@tkelman
Copy link
Contributor

tkelman commented Jun 3, 2017

@mortenpi oops, this makes linkcheck fail - should probably skip linkchecking on relative references

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants