Correctly report mkdocs theme name

[davidfischer]

The vast majority of docs built with mkdocs are in fact the readthedocs theme. However, some aren't. Here are some counter-examples:

• https://crafttweaker.readthedocs.io/en/latest/
• https://mcforge.readthedocs.io/en/latest/

[ericholscher]

Do we actually want to set this here? This will change what we're writing to the users config, which seems like we don't want to do that.

[davidfischer]

This user_config comes from their mkdocs.yml file. The only place it is written is to the READTHEDOCS_DATA variable in the docs. I believe we do in fact want to know if people are using mkdocs with a different theme.

[ericholscher]

We dump the mkdocs.yml back out below though, so this variable will exist in the mkdocs.yml we use to build the users project. Will having this unknown YAML key not mess with stuff?

[davidfischer]

Oh, I didn't fully realize what you meant. Ya, this should probably happen after the mkdocs.yml is written...

[ericholscher]

I thought we were forcing the RTD theme for mkdocs. I'm curious how these folks got around that: https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/doc_builder/backends/mkdocs.py#L154

[davidfischer]

They get around it by specifying a theme directory in their mkdocs.yml file. Here's an example: https://github.com/MinecraftForge/Documentation/blob/master/mkdocs.yml#L78

[ericholscher]

Looks good, assuming mkdocs doesn't blow up with a theme_name in the YAML during build.

[davidfischer]

I didn't fully understand what you meant previously (although it is clear in retrospect). I made a change to not write the name to mkdocs.yml while still writing it to readthedocs-data.js.

[humitos]

Looks good.

I noted a possible error depending on the value of the theme_dir

[humitos]

Since we are managing a path I think it's better to use os.path. Depending on the value of theme_dir this could fail:

>>> '/home/humitos/docs/theme'.rsplit('/')
['', 'home', 'humitos', 'docs', 'theme']
>>> '/home/humitos/docs/theme/'.rsplit('/')
['', 'home', 'humitos', 'docs', 'theme', '']
>>> 

in the second case we will endup with an empty string.

I suggest something like this:

>>> os.path.basename('/home/humitos/docs/theme/'.rstrip('/'))
'theme'
>>> os.path.basename('/home/humitos/docs/theme'.rstrip('/'))
'theme'

(you can achieve the same behaviour with your implementation just adding the rstrip --just in case)

[davidfischer]

I hesitate to use os.path because I suspect MkDocs always uses / regardless of what os.sep is.

However, your point on rstrip is well taken. I'll update.

[davidfischer]

Updated based on feedback...

[ericholscher]

I could also see this just being theme in practice, for example docs/custom/theme or something. Should we be slugifying the entire path name here, instead of just taking the last attribute? I guess for what we're doing it's close enough, but it does still feel lossy.