ultrabug / mkdocs-static-i18n

MkDocs i18n plugin using static translation markdown files
https://ultrabug.github.io/mkdocs-static-i18n/
MIT License
228 stars 38 forks source link

fix search plugin handling #203

Closed kamilkrzyskow closed 1 year ago

kamilkrzyskow commented 1 year ago

Fixes #202 Fixes #182 Closes #148

Hello again 👋,

I've introduced a new file with a function to get a plugin instance no matter if it's theme based or not. However, the algorithm is primitive it doesn't lookup exactly the name of the current theme, because I'm not sure if I can just use config["theme"]["name"] 🤔To be honest I like the "wider search scope" of the primitive approach more.

This function might be used with every plugin, but I doubt that any theme will introduce their own minify version etc., therefore I didn't refactor any plugin retrieval apart of the search.

The new material's theme search plugin also breaks the API, so I'm handling both the old entries and new _entries SearchIndex attributes. I added a fail-safe try/except too, in case some other theme creates their own search plugin with yet another API divergence😅

I run the tests in the repository, all passed, but I think all tests use the default theme. However, I've been using the initial fix in the Gothic Modding Community's docs for the past month, and no serious issue stuck out. Unfortunately, our alternative languages aren't supported by lunr.js, and with the current fix I get a warning 😞. The lang attribute is of the same type in the material plugin and default plugin, so everything should be fine 👌

kamilkrzyskow commented 1 year ago

I think that this PR is more "urgent" than the hreflang one tbh. What is missing here @ultrabug? Do I have to write tests for the material theme?

ultrabug commented 1 year ago

You're right, let's get this in.

ultrabug commented 1 year ago

Ok I'll fix the CI, thanks mate!

squidfunk commented 1 year ago

The new material's theme search plugin also breaks the API, so I'm handling both the old entries and new _entries SearchIndex attributes. I added a fail-safe try/except too, in case some other theme creates their own search plugin with yet another API divergence😅

You're using a private API – changes may happen at any time. It's a better idea to read the search_index.json file and process it then using the search index as I already mentioned in another comment.

kamilkrzyskow commented 1 year ago

I didn't run the linter locally 🤦thanks for solving the issue for me ✌️


It's a better idea to read the search_index.json file and process it then using the search index as I already mentioned in another comment.

I must've missed that other comment, or I didn't get the point at the time. The idea seems like a good solution 👍

ultrabug commented 1 year ago

Indeed @squidfunk ; As a matter a fact, some pieces of the plugin is actually forced to do the same on the mkdocs side...

@kamilkrzyskow if you can spare the time, a PR is welcome ofc

ultrabug commented 1 year ago

What I'm weary about is that AFAIK you changed the format of the search index in material @squidfunk ; am-I correct?

If so, we'll have to support two different JSON formats logic instead of two private API objects... not sure of the bargain.

squidfunk commented 1 year ago

What I'm weary about is that AFAIK you changed the format of the search index in material @squidfunk ; am-I correct?

Yes, as it's an entirely separate plugin now – it's not based on MkDocs search plugin anymore (previously was). For that reason, more is subject to change. Nonetheless, we will likely keep the structure of search_index.json, but add more fields in the near future. If you check our issue tracker, there are many, many requests related to extending search.