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

Allow plugins to override Page when populating navigation #3366

Closed
squidfunk opened this issue Aug 31, 2023 · 2 comments · Fixed by #3367
Closed

Allow plugins to override Page when populating navigation #3366

squidfunk opened this issue Aug 31, 2023 · 2 comments · Fixed by #3367

Comments

@squidfunk
Copy link
Sponsor Contributor

Following up on squidfunk/mkdocs-material#5909 (comment), I'm suggesting to adjust the logic that MkDocs implies to create Page objects from/in files, i.e., File objects. Currently, _data_to_navigation will always create (and thus overwrite) Page on everything that doesn't look like a Link or Section. Excerpt of the comment linked:

MkDocs will always generate either a Link or a Page for a file that is not excluded. As you've probably noticed, the blog plugin makes use of subclasses for pages of specific types, e.g. Post, View, etc., and relations among different concepts in the plugin are managed inside the instances of those subclasses. The problem is, that MkDocs will always generate a plain old Page instance, overwriting the more specific instance that has been generated by the plugin before after on_files and before on_nav. We could still solve it by splitting file generation and page instantiation, but this would make things much more complicated and non-linear, because during generation, we already assign categories to pages and vice versa, etc. We would need to split this into two stages, blowing up the code base of the plugin and also the runtime, as we're doing two passes. This is clearly not favorable from our perspective.

Moving forward: I believe that this problem can be mitigated with a very small but important change in MkDocs: if a File already defines a dedicated Page in file.page, MkDocs should just use that instead of creating a new one:

return file.page if isinstance(file.page, Page) else Page(title, file, config)

It's essentially a single line change in MkDocs that would make things much, much simpler for plugin developers. I've written several plugins now, and I repeatedly had to work around the oddities of this behavior.

Thus, I'm proposing to change the following line:

# Now: always create a page
return Page(title, file, config)

# Better: only create a page when one was not created before
return file.page if isinstance(file.page, Page) else Page(title, file, config)

Currently, the only way to work around this limitation is to overwrite Page.__class__ on the resulting instance in on_nav, e.g., like done in section-index, which, AFAIK, does not call __init__ or initializes instance variables:

page.__class__ = CustomPage
assert isinstance(page, CustomPage)
@squidfunk
Copy link
Sponsor Contributor Author

squidfunk commented Sep 3, 2023

We just got a report for a problem downstream that is blocked by this issue and would be fixed by the proposed solution:

We currently work around this issue with the following hack.

@ultrabug
Copy link
Member

ultrabug commented Sep 4, 2023

From a first look I don't see any downside in providing this simple patch to the page logic.

@oprypin would you agree?

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 a pull request may close this issue.

2 participants