Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in social plugin and add support for logo in custom_dir #5447

Merged
merged 3 commits into from
May 2, 2023

Conversation

kamilkrzyskow
Copy link
Collaborator

@kamilkrzyskow kamilkrzyskow commented Apr 30, 2023

Hi 👋,
I've been working on adding social cards to our documentation, and I've encountered a few minor bugs, while doing so.
The crash was related to a valid, but partially incomplete mkdocs.yml configuration:

File "C:\MyFiles\Gothic Modding\git\mkdocs-material\material\plugins\social\plugin.py", line 391, in _load_font
  name = theme["font"]["text"]
         ~~~~~~~~~~~~~^^^^^^^^
KeyError: 'text'
font:
    code: JetBrains Mono

Traceback copied after switching to the git repository theme installation, but I'm typically using the normal PyPi installation.

I've fixed it in a way to handle both font: false and font: {...} inputs, and it allows for the the text key to be missing.


Fixes #4920
Fixes #5128

I've read both of those issues and I kind of disagree that the issue is unfixable with the current architecture or that fixing it would require adding flags, which could lead to code debt in the future. IMHO the behaviour of override asset paths taking priority over docs assets is expected and fully correct, therefore it's natural for the user to use this directory to store "shared" / "static" assets. This approach is frequently used with multilingual documentations, even the currently most popular guide uses this approach. Therefore I ask you to reconsider 🙏

I've added a custom_dir path lookup. If a given path exists in the overrides directory, then it's used to generate the logo.


As a side note the Python script is in dire need of de-JavaScriptifying. Running black (with -l120 option) or some other tool to format the code could make it more readable ✌️ (mainly because my IDE wouldn't underline everything 😅). Considering your TypeScript background I was also surprised for the lack of type hints in the code, but then again they could add too much development overhead 🤔

@squidfunk
Copy link
Owner

Thanks for the PR! I'll check it asap. Note that I'm currently working on a ground up rewrite of the social plugin which is close to being finished. The new version will fix all of those issues: custom icons can now be easily used, and even completely custom and parametrizable layouts (via front matter and mkdocs.yml) are supported.

Since this will be exclusive to sponsors at first, this PR has great value as it fixes those issues in the community edition as well. Thank you for taking the time! As said, please give me some time to check it thoroughly.

@squidfunk squidfunk merged commit 5c57458 into squidfunk:master May 2, 2023
@squidfunk
Copy link
Owner

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants