GF font retrieve win (#7085)

[alexvoss]

Changed the way that font information is extracted once downloaded from GF. This avoids creating a temporary file and so resolves the issue with NamedTemporaryFile on Windows (#7085)

Tested this with our own documentation on Windows and MacOS.

[squidfunk]

Note that there's no necessity in upstreaming this to Insiders – I'll handle that immediately once we merge this PR.

[kamilkrzyskow]

LGTM 👍 However, I see the comment isn't formatted to make every line as long as possible to the 80 character limit, Martin could see an issue with that 👀

EDIT: Also I noticed this throughout the whole codebase, but I personally think that context managers should be as short and "focused" as possible. So only the font = ImageFont should be inside it, as other actions should work after the the context manager has been closed. So it's still approved by me, as a few more instructions in the with block won't impact anything, but if you're going to make changes perhaps do that too?

[kamilkrzyskow]

A new small issue for this PR #7118 as it's related to fonts

[squidfunk]

@kamilkrzyskow ha, you know me quite well now. Sorry, if I'm pedantic, just trying to keep everything as tidy as possible 😅 Regarding #7118 – can we solve this here as well, or should we create a new PR? I'm quite sure this is already solved in Insiders, but I'm not 100% sure.

[alexvoss]

Regarding #7118 – can we solve this here as well, or should we create a new PR?

That should be #7118 fixed as well. See if you like the solution. It should deal with (a) no font option (b) font: false (c) font option but no text font configured as well as (d) a font configuration with a text font configured.

[squidfunk]

Thanks, @alexvoss. It's actually a regression of 9.5.19, where I tried to make the social plugin work with other themes, see 6a761ed. The new tags plugin should not suffer from these problems. Could you please check if the social plugin still works with the mkdocs theme after this change? Here's almost a minimal reproduction.

[kamilkrzyskow]

The new code changes LGTM, also should work with the base theme as well. The changes in 6a761ed (which weren't documented with any comments btw ;^]) target the usage of direct indexing with [] into the theme, which I guess leads to a KeyError, because material's mkdocs_theme.yml defines them:

mkdocs-material/material/templates/mkdocs_theme.yml

Line 33 in ff49d74

font:

and the config file for the base theme doesn't:
https://github.com/mkdocs/mkdocs/blob/master/mkdocs/themes/mkdocs/mkdocs_theme.yml

There are some other cases in the codebase where direct indexing is used instead of the .get method.

mkdocs-material/src/plugins/social/plugin.py

Lines 134 to 137 in ff49d74

	# Retrieve palette from theme configuration
	theme = config.theme
	if "palette" in theme:
	palette = theme["palette"]

but are protected by a if name in theme conditional, therefore the compatibility with other themes / custom user configurations is upheld.

---

This line in the Blog plugin (Insiders too) should also be fixed for interoperability with the base mkdocs theme, as instead of language they use locale:

mkdocs-material/material/plugins/blog/plugin.py

Line 788 in ff49d74

locale: str = config.theme["language"].replace("-", "_")

[squidfunk]

@kamilkrzyskow You got me! 🫣 You're of course right. The code base of the tags and social plugin will be entirely replaced when the next funding goal is hit, which might explain my (subconscious?) sloppiness. And yes, the mkdocs_theme.yml defines the defaults, thus exposing the problem only in third-party themes. Making plugins compatible with other themes is something we just started to do, so some more work is necessary.

For this reason, I'd say we rather bandaid those plugins, as the new implementations are much cleaner and shouldn't suffer from the same problems. IMHO, we should concentrate our efforts on those implementations. If bugs arise, we'll of course fix them in the community edition as well, but rewriting them is not economical.

This line in the Blog plugin (Insiders too) should also be fixed for interoperability with the base mkdocs theme, as instead of language they use locale:

Yes, I agree, but that is an entirely different pit to fall in. Material's i18n implementation predates the one in MkDocs, and at some point, we should switch it, but that's something that is absolutely not a priority right now. When you specify language alongside locale, the blog plugin should work with third party themes, but we currently cannot switch the entire theme to use locale for lack of time. It's quite a big undertaking with breaking changes.

[squidfunk]

I've adapted the fix to the social plugin in Insiders (which already works for third party themes) in aeed812. I hope I'm not too pedantic, but I restructured the code a little and moved out the things of the context that don't need to be there.

I've realized too late, that I already committed that to master (wanted to discuss on a PR first), but I think the change should be safe and work. Could somebody verify before we merge this PR as well and issue a new release?

[alexvoss]

I restructured the code a little and moved out the things of the context that don't need to be there

Was wondering whether to do that but I decided that extracting a few variables could sit in the with block. It is not like a lot of work is getting done there that would delay closing the ByteIO. The advantage I saw was that the block shows that these lines belong together. Same is expressed by spacing, so in the end I don't think it matters much either way.

Could somebody verify before we merge this PR as well and issue a new release?

Am building our own documentation using Insiders and all seems well font-wise. Also tested the reproduction for #7118.

Am getting warnings from the RSS plugin, though. It seems to be looking for social cards images that do not exist as they are based on where the .md file is, not the eventual path to the blog post. Is this a known issue?

[squidfunk]

Thanks for testing, I guess we're ready to merge then. I guess it's a matter of taste what belongs where, but I think I saw @kamilkrzyskow note at some point that it's a best practice that contexts are kept as short as possible, but I might also be mistaken. The RSS plugin warning should be upstreamed.

[alexvoss]

best practice that contexts are kept as short as possible

In general, definitely. Might be good to make it muscle memory to do so as well. I still need to develop that.

The RSS plugin warning should be upstreamed

I added a reproduction to the issue Guts/mkdocs-rss-plugin#257

While at it I also stumbled across a difference in what the blog plugin expects a date to be and how the date-in-page-meta feature is documented for the RSS plugin but Guts has already committed changes that may sort this out with the next release, so I stopped short of writing it up as an issue.

[squidfunk]

@alexvoss Thanks! I don't think the mentioned upstream issue talks about the same error, it's a different error not related to Material for MkDocs but to all themes. More specifically:

INFO    -  DeprecationWarning: Do not access Theme._vars, instead access the keys of Theme directly.
             File "./venv/lib/python3.11/site-packages/mkdocs_rss_plugin/util.py", line 563, in guess_locale
               return mkdocs_config.get("theme")._vars.get("locale")
             File "./venv/lib/python3.11/site-packages/mkdocs/theme.py", line 87, in _vars
               warnings.warn(

[alexvoss]

@squidfunk that is not the one I ran into today and is something that should have been fixed a while ago. Are you running an up-to-date version of the RSS plugin? I found I was an on a really old one and was still getting this warning today, so I updated and this one is gone.

Edit: the version in our pyprojects.toml is 1.6 but the current is 1.12.1. If you think we should bump this I'd be happy to do the necessary testing. There is a tag for 1.12.2, so another release might be around the corner.

The RSS issue I was talking about is the "Remote image could not been reached" one.

[squidfunk]

Of course not 🫣 Just upgraded the plugin and now I see the remote image errors:

INFO    -  Cleaning site directory
INFO    -  Building documentation to directory: /Users/squidfunk/.workspace/squidfunk/repos/mkdocs-material/insiders/site
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/adding-a-badge-to-your-project.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/adding-a-badge-to-your-project.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/adding-a-badge-to-your-project.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/blog-support-just-landed.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/blog-support-just-landed.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/blog-support-just-landed.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/chinese-search-support.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/chinese-search-support.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/chinese-search-support.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/excluding-content-from-search.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/excluding-content-from-search.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/excluding-content-from-search.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/git-sparse-checkout.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/git-sparse-checkout.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/git-sparse-checkout.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/search-better-faster-smaller.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/search-better-faster-smaller.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/search-better-faster-smaller.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/sunsetting-gitter.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/sunsetting-gitter.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/sunsetting-gitter.png after 2 attempts. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/the-past-present-and-future.png. Trying again with GET and disabling SSL verification. Attempt: 1. Trace: HTTP Error 404: Not Found
WARNING:root:Remote image could not been reached: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/the-past-present-and-future.png. Trying again with GET and disabling SSL verification. Attempt: 2. Trace: HTTP Error 404: Not Found
ERROR:root:Remote image is not reachable: https://squidfunk.github.io/mkdocs-material/assets/images/social/blog/posts/the-past-present-and-future.png after 2 attempts. Trace: HTTP Error 404: Not Found
INFO    -  Doc file 'plugins/social.md' contains a link '../setup/changing-the-logo-and-icons.md#logo-image', but the doc 'setup/changing-the-logo-and-icons.md' does not contain an anchor '#logo-image'.
INFO    -  Doc file 'plugins/social.md' contains a link '../setup/changing-the-logo-and-icons.md#logo-icon-bundled', but the doc 'setup/changing-the-logo-and-icons.md' does not contain an anchor
           '#logo-icon-bundled'.
INFO    -  Doc file 'reference/content-tabs.md' contains a link '#anchor-links--or-even-me', but there is no such anchor on this page.
INFO    -  Doc file 'reference/content-tabs.md' contains a link '../publishing-your-site.md#with-github-actions-insiders', but the doc 'publishing-your-site.md' does not contain an anchor
           '#with-github-actions-insiders'.
INFO    -  Doc file 'setup/ensuring-data-privacy.md' contains a link '#example', but there is no such anchor on this page.
INFO    -  Documentation built in 7.90 seconds

Downgrading again 😅

[squidfunk]

We should currently not bump it. The version range we support is >=1.6, < 2, which is given by ~=1.6 (semver). If we bump it, we force the update upon all users and will probably receive a flood of issues with the remote image error. Let's wait for the fix, and then we can decide if we want to bump it, but I think we should be very careful doing that due to the recent discussions. Some users disagree with us limiting version ranges to tightly. Discussion is still ongoing.