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

Update material builtin plugins with patterns #7467

Closed
wants to merge 2 commits into from

Conversation

FredZinelli
Copy link
Contributor

@FredZinelli FredZinelli commented Aug 20, 2024

WIP

The patternProperties idea was definitely better!

I'm not really fond of the pattern for the strings, though. Maybe two const declarations would be better?
I think I should have either used two const, or maybe added a default value (if possible. It's my first time building json schema, as you probably guessed...) with the patterns, because for now, the user will loose the suggestions.

Your opinion?

@FredZinelli
Copy link
Contributor Author

FredZinelli commented Aug 20, 2024

So, in fact... It's the use of patternProperties (edit: any use of patterns, in fact) that removes the plugin from the suggestions list. Maybe not the way to go, in that case?


edit:

Ok, I made a smaller example of the changes using $defs below, on the typeset schema. This allows to keep the suggestions

Original structure:

{
  "$schema": "https://json-schema.org/draft-07/schema",
  "title": "Built-in typeset plugin",
  "oneOf": [
    {
      "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/",
      "const": "typeset"
    },
    {
      "type": "object",
      "properties": {
        "typeset": {
          "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/",
          "type": "object",
          "properties": {
            "enabled": {
              "title": "Enable plugin",
              "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/#config.enabled",
              "type": "boolean",
              "default": true
            }
          },
          "additionalProperties": false
        }
      },
      "additionalProperties": false
    }
  ]
}

Modified structure:

{
  "$schema": "https://json-schema.org/draft-07/schema",
  "title": "Built-in typeset plugin",
  "oneOf": [
    {
      "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/",
      "const": "typeset"
    },
    {
      "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/",
      "const": "material/typeset"
    },
    {
      "type": "object",
      "properties": {
        "typeset": {
          "$ref": "#/$defs/typeset"
        },
        "material/typeset": {
          "$ref": "#/$defs/typeset"
        }
      },
      "additionalProperties": false
    }
  ],
  "$defs": {
    "typeset": {
      "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/",
      "type": "object",
      "properties": {
        "enabled": {
          "title": "Enable plugin",
          "markdownDescription": "https://squidfunk.github.io/mkdocs-material/plugins/typeset/#config.enabled",
          "type": "boolean",
          "default": true
        }
      },
      "additionalProperties": false
    }
  }
}

This gives the following suggestions:
image

Notes:

  1. the markdownDescription could be moved outside of the "oneOf", to reduce duplication.
  2. since the suggestions come from the object types, the consts could be replaced with a pattern, there. Edit: actually, no. using any kind of pattern makes the suggestions disappear... :/

@squidfunk
Copy link
Owner

Thanks for the PR. I'm still on the fence whether we want to support the material/* syntax here, since it is not necessary, and now those suggestions are popping up in the IDE. It might lead to more questions on our side (we have many non-/low-tech users), as there are now two ways to achieve the same thing.

However, you can change your theme's entrypoint and list the Material plugins with your prefix, which doesn't mandate a change on our side, specifically here:

https://github.com/squidfunk/mkdocs-material/blob/47f7f631043b3d862a7dd88dc9f742b0e143c47b/pyproject.toml#L77C1-L85C60

Something like the following should work:

[project.entry-points."mkdocs.plugins"]
"foobar/blog" = "material.plugins.blog.plugin:BlogPlugin"
...

If your theme's called foobar, just re-export all plugins with foobar. Closing, as this can be resolved on your side.

@squidfunk squidfunk closed this Aug 20, 2024
@squidfunk
Copy link
Owner

By the way, the big upside is, that users can use a configuration for material with your theme without changes.

@FredZinelli
Copy link
Contributor Author

Yes, on second thoughts, it's probably better like this.

I'll check these entry points things. Thx for the suggestion.

👍

@FredZinelli
Copy link
Contributor Author

FredZinelli commented Aug 20, 2024

By the way, the big upside is, that users can use a configuration for material with your theme without changes.

Unfortunately, it doesn't seem to work, at least for the search plugin : the search bar is never showing up, whatever the entry points and the declaration in mkdocs.yml:plugins (except with material/search, ofc).
I remember reading issues/discussions where you said its internals are complex and the plugin registration name actually problematic (search vs material/search). So maybe it's the only one which triggers problems, but that's the only one I currently use... ':) . Too bad...

Cheers

@squidfunk
Copy link
Owner

squidfunk commented Aug 20, 2024

There are a few mentions in the templates that you'd need to override.

Replace them with your theme name, and it should work. I'm sorry that this is so complicated, but to be honest, MkDocs doesn't make it particularly simple for themes to ship plugins. We'll look into making customization simpler as well, as part of our latest efforts.

@FredZinelli
Copy link
Contributor Author

Wow, so obvious (and not, at the same time... XD ). Works great, thanks!

I'll proceed based on this and provide some updated schema on my side.
I'll also fork again to remove the plugin related to my theme: that wouldn't make any sense anymore to have it in your schema while the plugin cannot go without my theme, which will propose its own schema overrides...

Thanks a lot for your help!

@squidfunk
Copy link
Owner

Glad I could help and we could conserve the steps for anyone trying to achieve the same 🚀

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

Successfully merging this pull request may close these issues.

2 participants