Guts/mkdocs-rss-plugin

Deprecation of Theme._vars

Closed this issue · 8 comments

alexvoss commented

Hi all,

using the HEAD versions of mkdocs today, I stumbled across this:

mkdocs serve --watch-theme
INFO    -  Building documentation...
INFO    -  DeprecationWarning: Do not access Theme._vars, instead access the keys of Theme
           directly.
             File
           "/Users/avoss/src/mkdocs-material/devpt/venv/lib/python3.11/site-packages/mkdocs_rss_plugin/util.py",
           line 624, in guess_locale
               locale = mkdocs_config.get("theme")._vars.get("locale")
             File "/Users/avoss/src/mkdocs-material/devpt/mkdocs/mkdocs/theme.py", line
           81, in _vars
               warnings.warn(

This seems to have been introduced in MkDocs 1.5.0 (see bottom of the list of changes)? Not sure why I have not stumbled across this before...

Did a quick search and it seems there is one other place where this would need to be changed.

Hope this helps and that it is an obvious one-liner to fix.

Best wishes,

Alex

Guts commented

Hi @alexvoss ,

Thanks for reporting. I try to fix it this week.
Or maybe you want to take the opportunity to make a PR? You've already did the 80% 😉

alexvoss commented

I'll see if i can do a PR tomorrow, will put it on my todo list.

  • 👍1
alexvoss commented

Please bear with me. I want to see if I can get rid of the second usage of _vars as well but from a brief glance at the MkDocs code it is not entirely clear that a Theme as a language attribute. I also want to make sure both changes are covered by tests.

alexvoss commented

Right, I think I have got somewhere. I did make changes to util.py to get rid of the usages of _var but on the way went down the rabbit-hole of trying to debug this by running the tests. I just want to document this here because it took me a long time to figure it out as I kept suspecting dead code and broken tests first:

To debug, you need to run pytest with --no-cov argument to enable debugging. This is not just a VSCode issue, it seems, so may be worth adding to the developer documentation?

I have rewritten the existing code to have what I think is equivalent functionality but slightly streamlined logic that helps with debugging. Got rid of the use of _vars and replaced it with the use of square brackets. Will fork, commit and do a PR now.

I do suspect that the test for language is dead code because the Theme class seems to always set a locale now? We can deal with this in a next step if I am not holding the wrong end of the stick on that.

Guts commented

Please bear with me. I want to see if I can get rid of the second usage of _vars as well but from a brief glance at the MkDocs code it is not entirely clear that a Theme as a language attribute. I also want to make sure both changes are covered by tests.

The material theme has a language attribute: https://squidfunk.github.io/mkdocs-material/setup/changing-the-language/#site-language

alexvoss commented

Right, I think I missed a comment in your code to the effect that these come from the theme.

alexvoss commented

Thanks for this. I did a quick smoke test with the Material blog-rss integration example and it seems fine.

  • 👍1