Reflect language in lang attribute. #443
Conversation
|
Hi thanks for the patch I opened a PR a while ago with the same idea (#406) However @Minecrell pointed this out:
As a solution, we found that you can use http://jinja.pocoo.org/docs/2.9/templates/#replace but I never got around to updating the PR. If you want to talk you can poke me in IRC (sorry I missed your message). |
|
Aha, don't know how I missed that one. Thanks, I'll experiment with replacing the underscore. |
|
This change does the job. Also tidied up the syntax a little bit. |
There was a problem hiding this comment.
Looks good to me, would like feed back from either @agjohnson or @ericholscher before committing.
| @@ -6,10 +6,11 @@ | |||
| {%- else %} | |||
| {%- set titlesuffix = "" %} | |||
| {%- endif %} | |||
| {%- set lang_attr = 'en' if language == None else (language | replace('_', '-')) %} | |||
There was a problem hiding this comment.
I wonder if this catches the proper language in most cases. In particular, what happens if language is undefined? I believe jinja has a check for this. Have you tried building sets of docs with this patch and different language variables defined?
There was a problem hiding this comment.
I've checked the output of
sphinx-build demo_docs/source demo_docs/build
sphinx-build demo_docs/source demo_docs/build -A language=pt_BR
In the first case language is None so en is used. I can update it to handle things like empty string if we're expecting people to pass -A language=. I was taking a cue from the official basic theme which just checks None-ness (though it uses is none which equivalent but looks like the more "correct" way to do it, so happy to update the syntax there).
https://github.com/sphinx-doc/sphinx/blob/master/sphinx/themes/basic/layout.html#L120
|
Thanks! |
The basic Sphinx templates check
languageand use it to set<html lang>, so I think it makes sense to do this here. A few sites like Blender documentation have multiple languages, and at the moment Google will return results from all the languages even when asking for English results. With a properlangI think it will be able to return appropriate results.I'm not a Jinja ninja so let me know if there's a more concise way to do the if/else. I've built the demo site to check that it works (defaults to
en, and changes when-A language=fris passed)