ENH: Add logo_link theme option

[leonarduschen]

Closes #245

Add logo_link option to specify the page linked to logo (default is master_doc)

logo_link is treated as a URL if it starts with "http" otherwise treat it as RST doc.

[choldgraf]

What is the expected behavior if somebody changes their master_doc to be something other than index? Won't this result in a broken link? Or will an index file always be created?

[leonarduschen]

I think it would still be okay as long as that person writes the following (will confirm this locally)

html_additional_pages = {
    'index': '<template>.html',
}

which as far as I know is how people usually do it when they make custom HTML landing page.

That said, it'll probably result in a broken link if the project does not have an index page at all. Do you think it is a better idea to add an option to link logo to other than master_doc in theme.html ?

[choldgraf]

yeah I'd rather provide a theme option to over-ride the logo link...I think that's a bit less-hacky than asking people to define a custom template mapping

[jorisvandenbossche]

@leonarduschen thanks for looking into this!

One thing I am wondering: how do other themes handle this? Eg readthedocs theme also has a logo that points to the "home"/"index" page.

[jorisvandenbossche]

Hmm, seems they are also using master_doc .. https://github.com/readthedocs/sphinx_rtd_theme/blob/1d856256676d2b801a12c30e4a849c5abc466da9/sphinx_rtd_theme/layout.html#L119

[leonarduschen]

Hmm, seems they are also using master_doc .. https://github.com/readthedocs/sphinx_rtd_theme/blob/1d856256676d2b801a12c30e4a849c5abc466da9/sphinx_rtd_theme/layout.html#L119

Yep it seems so. Alabaster does too https://github.com/bitprophet/alabaster/blob/master/alabaster/about.html

But I guess it can't hurt to add a logo theme option though (instead of the problematic original idea to change it to index)

[leonarduschen]

@choldgraf

yeah I'd rather provide a theme option to over-ride the logo link...I think that's a bit less-hacky than asking people to define a custom template mapping

Done!

[leonarduschen]

@choldgraf I have tested locally and can confirm it works for both external link and local doc.

[leonarduschen]

ping

[jorisvandenbossche]

@leonarduschen sorry for the slow follow-up, but this looks good to me!

Could you add a small section about this option in configuring.rst ?

[leonarduschen]

done!

[choldgraf]

Suggested change

	The logo links to ``master_doc`` by default. If you'd like to link to other page
	The logo links to ``master_doc`` (usually the first page of your documentation) by default. If you'd like to link to another page

[leonarduschen]

Agree on "other page" to "another page" but the logo really links to master_doc by default though and sometimes master_doc is not the landing page (e.g. numpy dev docs).

[choldgraf]

hmmm - fair enough, what do you think about the new language suggestion? The thing is that most users don't know what master_doc is so I prefer using terminology that the "average" user will likely understand

[choldgraf]

another thing we could do instead is include a link to the Sphinx documentation for master_doc

[leonarduschen]

I think the new language suggestion is pretty informative!

[leonarduschen]

@jorisvandenbossche let me know if you'd like me to change anything :)

[mattip]

ping

[jorisvandenbossche]

Sorry for the slow follow-up, @mattip thanks for the ping, and @leonarduschen thanks for the updates! Looking good!

[bryevdv]

Just confirming, this is not in a release yet? This option is described in the published docs, but I get an error about logo_link being an unknown theme option when I try to use it, and I can't actually see this PR in the 0.4.x commit history, either. (But it is in the master commit history) Will this be released soon?

[jorisvandenbossche]

This option is described in the published docs,

What you link to are the development docs .. (we should maybe link to the stable docs on the github landing page / make it the default readthedocs url).
But so, indeed not yet in a release. We should urgently release, but there are a few things I want to finish before doing that (but have been a bit busy with other work lately). I just mentioned it on the bokeh issues as well, but for testing out the upstream theme for the bokeh docs, I would suggest to use the development version for now until we do a release (you can pip install it from the git url, and eg put that in an environment.yml file)

[bryevdv]

Ah yes, I just clicked through from the link in the GitHub README (also I assumed "latest" means "latest release" but forgot RTD has a strange (to me) convention about that). Thanks for clarifying!

[choldgraf]

@jorisvandenbossche is there a reason you don't want to release? I am always worried when I see sentences like

We should urgently release, but there are a few things I want to finish before doing that

Cutting a release only takes like 2 minutes, so we should just make a release unless there's a strong reason that we want to block it!

[leonarduschen]

I don't know much about release workflows but if QA is the concern I can quickly write some tests to cover this feature

[jorisvandenbossche]

@jorisvandenbossche is there a reason you don't want to release? I am always worried when I see sentences like

Actually yes ;) #312 needs to go in, because I don't want to have people start relying on variables names we are going to change. Now, that's a trivial PR and I rebased it now, so I can release later this evening.

[bollwyvl]

👍 to progress, I'll also get #294 back in fighting shape.

"latest" means "latest release" but forgot RTD has a strange (to me) convention about that

Yeah, perhaps adjusting the RTD workflow to reflect that, and having to Push The Button to "activate" a tag as a latest would reduce the dissonance...