bashtage / sphinx-material

A material-based, responsive theme inspired by mkdocs-material
Other
312 stars 91 forks source link

Rebase on new version of mkdocs-material #96

Closed jbms closed 2 years ago

jbms commented 3 years ago

This is rebased on version 5bda328145df756afb27362f507591b17b091474 of mkdocs-material (master as of 2021-03-01).

TODO:

Fixes #90 (I think) Fixes #89 Fixes #44 Fixes #71 Fixes #92

pep8speaks commented 3 years ago

Hello @jbms! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 34:1: E265 block comment should start with '# ' Line 52:5: E265 block comment should start with '# ' Line 110:9: E122 continuation line missing indentation or outdented Line 111:9: E122 continuation line missing indentation or outdented Line 112:9: E122 continuation line missing indentation or outdented Line 113:9: E122 continuation line missing indentation or outdented Line 118:5: E122 continuation line missing indentation or outdented Line 120:9: E122 continuation line missing indentation or outdented Line 121:9: E122 continuation line missing indentation or outdented Line 122:9: E122 continuation line missing indentation or outdented Line 123:9: E122 continuation line missing indentation or outdented Line 128:5: E122 continuation line missing indentation or outdented Line 211:1: E305 expected 2 blank lines after class or function definition, found 1

Line 132:9: E722 do not use bare 'except'

Line 6:1: F401 'typing.Optional' imported but unused Line 6:1: F401 'typing.Union' imported but unused Line 6:1: F401 'typing.Sequence' imported but unused Line 6:1: F401 'typing.Tuple' imported but unused Line 22:1: F401 '.autodoc_property_type' imported but unused Line 193:21: W503 line break before binary operator

Line 3:1: F401 'typing.Union' imported but unused Line 15:1: E302 expected 2 blank lines, found 1 Line 108:54: E701 multiple statements on one line (colon)

Line 5:1: F401 'typing.List' imported but unused Line 8:1: F401 'docutils.parsers.rst.directives' imported but unused Line 18:24: E701 multiple statements on one line (colon) Line 20:19: E701 multiple statements on one line (colon) Line 24:17: E701 multiple statements on one line (colon)

Line 3:1: F401 'typing.Set' imported but unused Line 16:28: E701 multiple statements on one line (colon) Line 175:18: E701 multiple statements on one line (colon) Line 246:51: E701 multiple statements on one line (colon) Line 249:27: E701 multiple statements on one line (colon) Line 251:27: E701 multiple statements on one line (colon) Line 270:37: W504 line break after binary operator Line 271:74: W504 line break after binary operator Line 278:28: E701 multiple statements on one line (colon) Line 310:9: E123 closing bracket does not match indentation of opening bracket's line Line 330:13: E123 closing bracket does not match indentation of opening bracket's line Line 345:5: F841 local variable 'theme_options' is assigned to but never used

Line 9:1: E302 expected 2 blank lines, found 1 Line 20:91: E501 line too long (98 > 90 characters) Line 23:69: E701 multiple statements on one line (colon) Line 41:91: E501 line too long (96 > 90 characters) Line 54:9: F841 local variable 'fieldbody' is assigned to but never used

Line 24:13: W503 line break before binary operator

Comment last updated at 2021-09-25 00:20:35 UTC
ghost commented 3 years ago

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that. Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

jbms commented 3 years ago

One other thing: I did not do this yet, but it may be useful to designate the relevant commit from mkdocs-material as an additional parent of this commit. That way it is easier to merge in later changes from there. However, the downside is that it will pull in the entire history of mkdocs-material, which is somewhat large due to having a lot of generated data and icon copies.

Also, currently I have excluded the generated files from the repository. That does mean users have to use node.js to build the generated files when installing from the git repository, though. You could have a CI workflow that builds the generated files and pushes them to another branch --- that would avoid polluting the main branch but still allow users to install the latest version with needing node.js.

jbms commented 3 years ago

Note: I'd suggest when reviewing this that you also look at the diff from mkdocs-material, since that is much smaller than the difference from the sphinx-material master.

bashtage commented 3 years ago

Thanks for this. It will take a while to get it all done.

Is there any chance it would be better to have a submodule for mkdocs material with a specific commit?

jbms commented 3 years ago

To make it easier to review the changes, I split my changes into several individual commits. Additionally, I added the upstream mkdocs-material commit on which these changes are based as a merge parent.

Unfortunately that does mean this pull request shows every commit from mkdocs-material --- you can find my commits at the end, starting from "Pull non-excluded files from mkdocs-material/master". You can look at just those commits to see the changes I had to make, which are relatively minor except for the search integration.

I think this approach will work reasonably well. The only downside is that your repository will now be at least as large as the mkdocs-material repository --- and that repository is somewhat large due to it containing a bunch of copied icons and generated javascript/css bundles that are re-generated on every commit. But I don't think there is a better way.

I don't think git submodules would work that well, because some changes need to be made to the mkdocs-material files --- they at least some of them can't just be referenced without modification.

Because it is slightly tricky to do the merging, I also added a tools/merge_from_mkdocs_material.py script that can be used to pull in changes from mkdocs-material in the future. Basically the issue is that if you just try to do a regular git merge to pull in changes from mkdocs-material, you will get conflicts on all of the files that have been excluded from being imported from the mkdocs-material repository (like the generated files, and other files not applicable to this project). The script works around that problem by creating an additional intermediate commit that deletes all of the excluded files from mkdocs-material before performing the merge.

pradyunsg commented 3 years ago

Ooo! This is neat!

@jbms you seem to have done most of same things as I did in https://github.com/pradyunsg/sphinx-mkdocs-theme, but significantly more thoroughly; albeit in a theme-specific manner! ^.^

jbms commented 3 years ago

@pradyunsg Interesting, hadn't seen that.

Certainly there is an advantage in supporting any mkdocs theme rather than just mkdocs-material, though some theme-specific changes seem to be inherently necessary to account for mismatches between the Sphinx and mkdocs document models (e.g. mkdocs doesn't expect pages to have sub-pages in the toc), search support, and support for Sphinx "object descriptions" which don't exist in mkdocs.

Currently I'm still working on revising this theme to add better styling for object descriptions, inspired by your own Furo theme and by pdoc3.

I'm ultimately planning to use this theme for the documentation for https://github.com/google/tensorstore

In general I very much like the power of sphinx, and it provides a lot of useful components, but a lot of assembly tends to be required to get a nice result...

It would be great to avoid duplicated effort between this theme and your sphinx-mkdocs-theme extension, but I'm also not sure how feasible it is to accomplish what I want to accomplish with this theme with an additional layer underneath.

pradyunsg commented 3 years ago

some theme-specific changes seem to be inherently necessary to account for mismatches between the Sphinx and mkdocs document models (e.g. mkdocs doesn't expect pages to have sub-pages in the toc)

Maybe? I was planning to enforce that as "nesting on the top level needs to be a captioned ToC". That realistically covers the whole toctree situation. :)

search support

I got a very neat (and hacky) solution working for this: populate a Lunr.js index from the Sphinx documents! :)

object descriptions

I imagine this is the API documentation stuff? Yea, this one is tricky. This is basically why I abandoned this effort, since there was no longer a way to do things in a general way. :)

jbms commented 3 years ago

On Wed, Mar 31, 2021, 22:35 Pradyun Gedam @.***> wrote:

some theme-specific changes seem to be inherently necessary to account for mismatches between the Sphinx and mkdocs document models (e.g. mkdocs doesn't expect pages to have sub-pages in the toc)

Maybe? I was planning to enforce that as "nesting on the top level needs to be a captioned ToC". That realistically covers the whole toctree situation. :)

search support

I got a very neat (and hacky) solution working for this: populate a Lunr.js index from the Sphinx documents! :

I looked into that approach a bit. I haven't done comparisons of the actual searching effectiveness, but I like how the sphinx index works better: the lunr index anyways contains the full text content of the entire site, and if a pre built index is used is several times larger than that. The sphinx index appears to be more compact (assuming most terms appear multiple times in a document) since it only stores (docid, term) associations. It also knows about object descriptions, so they can be handled specially. The downside to the sphinx index is that you have actually fetch pages in order to extract snippets and section information.

object descriptions

I imagine this is the API documentation stuff? Yea, this one is tricky. This is basically why I abandoned this effort, since there was no longer a way to do things in a general way. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bashtage/sphinx-material/pull/96#issuecomment-811654231, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAEJ2QF4P7VWHLKPOWPAFDTGQA3PANCNFSM4YOKXORQ .

nejch commented 3 years ago

@jbms great to see work on this, I was just looking at Furo and this theme to refresh some Sphinx docs.

Just another idea in case it works for you: our org is also basing a (mkdocs) theme on the upstream mkdocs-material, and have decided to add mkdocs-material as a git submodule in the repo, with only customizations added in the project and integrated/patched as part of the theme build.

You can then update the submodule continuously using dependabot/renovate, and test that it still works against upstream, with smaller deltas that are probably easier to resolve. It should be much cleaner than vendoring all the code long-term (from a quick glance it seems like you've had to copy/paste a lot of upstream files that might not be relevant for you).

Of course that's probably much easier to do with mkdocs-mkdocs customization than applying an mkdocs upstream theme to a sphinx theme. Just wanted to add this here in case you find it feasible and makes your life easier later down the road. But maybe this is more a topic for maintainers here (@bashtage?).

jbms commented 2 years ago

As an update, my own work on this effort is now pushed out here, currently embedded within the tensorstore repository itself: https://github.com/google/tensorstore/tree/master/docs/tensorstore_sphinx_material

You can see the generated documentation here to see the theme in action: https://google.github.io/tensorstore/

In particular, refer to the API documentation section, which required the most significant sphinx customizations: https://google.github.io/tensorstore/python/api/index.html

The theme adheres to mkdocs-material extremely closely, but the API documentation styling is largely my own invention, inspired by pdoc3 (and https://hikari-py.github.io/hikari/hikari/), and other sphinx themes like Furo, as mkdocs-material does not itself offer anything specific to API documentation.

I have kept up to date with upstream mkdocs-material. Currently the theme requires sphinx 3, but I also have some pending changes that I expect will be pushed out within a week or so to update to Sphinx 4 and to the latest mkdocs-material.

I would love to be able to upstream these changes back into sphinx-material and/or sphinx itself as appropriate, but I'm not sure what the best way forward is.

I can create an updated version of this pull request, but before spending more time on that it would be helpful to know if the direction I've taken things is one that @bashtage or others are interested in.

swarnat commented 2 years ago

Just my 2 cents: Your updates push the sphinx-material to a much better level, because it extend some needed functions and cut some useless. And I'm happy you continue your backports. :) Thanks! Generally you combine the power of two communities.

The only problem we had is the migration from this repo to your version. Maybe this is getting easier with your example documentation.

pradyunsg commented 2 years ago

FWIW, if @bashtage doesn't respond soon, it might be worth making this PR into a separate project that's effectively a better maintained fork of this project.

pradyunsg commented 2 years ago

Alternatively, if you really like the sphinx-material name and @bashtage genuinely is not reachable, PyPI's name retention policies allow transfer of projects for continued maintenance of abandoned projects.

https://www.python.org/dev/peps/pep-0541/#continued-maintenance-of-an-abandoned-project

bashtage commented 2 years ago

Shoukd decide whether this should be a stand alone theme or if someone want to take up full time maintenance. This was always a side project for me

bashtage commented 2 years ago

@pradyunsg this is not an avoiding project. There was a release last week. This update changes a lot and I need a clean block of time to figure out if these are welcome changes for the intended use, e.g., statsmodels.org

jbms commented 2 years ago

It sounds like there is definitely some interest in this theme, but it is unclear who has time to actually maintain it :)

I think I can commit to keeping it synced with upstream mkdocs-material. (I just pushed to the tensorstore repository an update that makes it work with sphinx 4 and syncs with newer mkdocs-material changes.)

However, I don't think I can really take on the additional package maintenance work of updating the documentation and examples, making releases, etc.

Also it would be great to get some help trying to reduce the amount of monkey patching required by merging the necessary fixes into upstream sphinx (see this issue I filed with sphinx https://github.com/sphinx-doc/sphinx/issues/9523).

Finally, for the tensorstore documentation there are some additional customizations that currently are partially separated from the theme itself: https://github.com/google/tensorstore/tree/master/docs/tensorstore_sphinx_ext

Most notably this includes a new implementation of autosummary and some monkey patching of autodoc in order to support pybind11 better. However, those changes aren't completely independent of the theme code --- for example there are some style rules in the theme that are only used by the new autosummary.

Currently the tensorstore_sphinx_ext code does contain some tensorstore-specific stuff, but that could be factored out easily enough.

However, I'm not clear on where that should go. Ideally some of it could go into upstream sphinx --- for example, the support for pybind11 overloaded functions. Potentially the new autosummary could live as part of sphinx-material. There is also a new json domain for documenting json schemas that integrates with some of the new theme functionality but is otherwise unrelated but might also be useful to some people.

astrojuanlu commented 2 years ago

Comment from the peanut gallery: cannot speak for @bashtage , but I think this would be way more manageable if it was split into smaller pull requests. There are more than 3 000 commits and the diff says +28,564 −8,064 - if I had to review and merge this, I would probably be scared as hell and need a huge amount of mental energy. Not to mention that possibly there would be several review cycles.

mhostetter commented 2 years ago

@jbms I'm blown away! The way you have styled your tensorstore website, with the new Material for Mkdocs theme and modified Sphinx autosummary, is just as I've always wanted. Especially the new autodoc/autosummary feature. I would love to see this finally get merged or break off into a standalone project.

I'm trying to build my FOSS Python package's docs off of your fork. (Still working through a couple of npm issues, which are a little tricky because I'm not a web dev)

@jbms, please don't give up on this. :pray:

2bndy5 commented 2 years ago

ugh. I'm now getting an error when building my docs with this modified theme:

Extension error (sphinx_material):
Handler <function html_page_context at 0x7f78f339e160> for event 'html-page-context' threw an exception (exception: <class 'sphinx_material._TocVisitor'> visiting unknown node type: title)

it's so terse I was hoping @jbms might have some insight

jbms commented 2 years ago

@2bndy5 I had not updated my branch in a while, and it did not support newer versions of sphinx.

I just pushed out an updated version based on the latest changes in the tensorstore repository. It should now build with the latest sphinx and docutils.

2bndy5 commented 2 years ago

@jbms Thanks for the response (I hadn't considered my installed version of Sphinx which is v4.2.0). I just re-installed the theme from git and tried to re-build my docs, but hit this error:

Exception occurred:
  File "/mnt/c/Users/ytreh/Documents/GitHub/env/lib/python3.8/site-packages/sphinx_material/autodoc_property_type.py", line 52, in add_directive_header
    retann = self.retann or get_property_return_type(self.object)
NameError: name 'get_property_return_type' is not defined

It might be worth mentioning that my docs don't use auto-summary


Assuming I can get this to work, I'm willing to throw my hat in the ring for possible maintainers... I'm just hesitant because I clearly don't know all the mechanics being used. I'm willing to learn as I go (one of the great opportunities in open-source contributions)! I mention this because I suspect this modified theme may need a home of its own (this way @bashtage isn't swamped) - we could call it sphinx-material2 or something.

jbms commented 2 years ago

I just pushed an update that fixes that problem.

2bndy5 commented 2 years ago

@jbms yes indeed it did fix that problem. Thank you! My docs are now building locally.

2bndy5 commented 2 years ago

I should probably mention that I get an error on consecutive builds (I'm now playing with the theme's "features" 😃 )

looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... failed

Exception occurred:
  File "/mnt/c/Users/ytreh/Documents/GitHub/env/lib/python3.8/site-packages/sphinx/search/__init__.py", line 283, in load
    index2fn = frozen['docnames']
KeyError: 'docnames'
The full traceback has been saved in /tmp/sphinx-err-kiec47f9.log, if you want to report the issue to the developers.

deleting the built docs before re-building fixes this error.

2bndy5 commented 2 years ago
jbms commented 2 years ago

I'm not seeing the issue with code blocks scrolling horizontally. That seems to work for me, both in desktop Chrome when "mobile device mode" is selected in dev tools, and in firefox and chrome on android. I don't have a convenient way to test on iOS/Safari though.

As far as the tables, the data class is added by extending the HTML translator -- refer to sphinx_material/__init__.py. Are you somehow overriding the html translator? You can try building the documentation in this repository itself, under the docs/ directory. The data class seems to be added correctly there on the "Specimen" page.

Regarding the failure with consecutive builds: for the tensorstore documentation it is set up to always do a clean build, so I hadn't noticed previously. The consecutive build error is related to the search index. One of my modifications was to strip out some unnecessary data included in the search index, that was not actually used for searching (and unnecessarily increased its size). Some other changes are also made to simplify the client-side searching. Refer to search_adapt.py. However, I did not realize that for incremental builds, it reads back the search index, and these fields are required for incremental builds. A proper fix, I think, would be to write the incremental build search data to a separate file, so that it doesn't bloat the actual search index included in the website. For now I would suggest just doing a clean build each time, as there may be other things in the theme that are incompatible with incremental builds.

2bndy5 commented 2 years ago

You can try building the documentation in this repository itself, under the docs/ directory

that's exactly what I'm doing:

(env) brendan@MY-PC:/path/to/repo/root/docs$ sphinx-build -E -W . _build

Are you somehow overriding the html translator?

Not sure how I would override the HTML translator, but I should mention that I'm using a custom pygment style for highlighting the code blocks (I never liked the default offerings).

# pygment custom style
# --------------------------------------------------

class DarkPlus(Style):
    """A custom pygment highlighting scheme based on
    VSCode's builtin `Dark Plus` theme"""

    background_color = "#1E1E1E"
    highlight_color = "#ff0000"
    line_number_color = "#FCFCFC"
    line_number_background_color = "#202020"

    default_style = ""
    styles = {
        Text: "#FEFEFE",
        Comment.Single: "#5E9955",
        Comment.Multiline: "#5E9955",
        Comment.Preproc: "#B369BF",
        Other: "#FEFEFE",
        Keyword: "#499CD6",
        Keyword.Declaration: "#C586C0",
        Keyword.Namespace: "#B369BF",
        # Keyword.Pseudo: "#499CD6",
        # Keyword.Reserved: "#499CD6",
        Keyword.Type: "#48C999",
        Name: "#FEFEFE",
        Name.Builtin: "#EAEB82",
        Name.Builtin.Pseudo: "#499DC7",
        Name.Class: "#48C999",
        Name.Decorator: "#EAEB82",
        Name.Exception: "#48C999",
        Name.Attribute: "#569CD6",
        Name.Variable: " #9CDCFE",
        Name.Variable.Magic: "#EAEB82",
        Name.Function: "#EAEB82",
        Name.Function.Magic: "#EAEB82",
        Literal: "#AC4C1E",
        String: "#B88451",
        String.Escape: "#DEA868",
        String.Affix: "#499DC7",
        Number: "#B3D495",
        Operator: "#FEFEFE",
        Operator.Word: "#499DC7",
        Generic.Output: "#F4DA8B",
        Generic.Prompt: "#99FFA2",
        Generic.Traceback: "#FF0909",
        Generic.Error: "#FF0909",
        Punctuation: "#FEFEFE",
    }

def pygments_monkeypatch_style(mod_name, cls):
    """function to inject a custom pygment style"""
    cls_name = cls.__name__
    mod = type(__import__("os"))(mod_name)
    setattr(mod, cls_name, cls)
    setattr(pygments.styles, mod_name, mod)
    sys.modules["pygments.styles." + mod_name] = mod
    pygments.styles.STYLE_MAP[mod_name] = mod_name + "::" + cls_name

pygments_monkeypatch_style("dark_plus", DarkPlus)
# The name of the Pygments (syntax highlighting) style to use.
pygments_style = "dark_plus"

Besides that, I did disable adding any extra css in conf.py

2bndy5 commented 2 years ago

I also noticed the toc is cut short on every rst file (cross-linking still works though)

.. examples.rst
chapter 1
===============
section a
------------
paragraph 1
************
section b
-------------

.. gets cut off here
chapter 2
==================
2bndy5 commented 2 years ago

commented out all that custom pygment stuff and rebuilt... code-blocks still don't have horizontal scrolling and the code-block bg is statically set, but adding the following fixed the bg problem

.md-typeset pre {
 background-color: var(--md-code-bg-color);
}
jbms commented 2 years ago

Regarding your TOC issue, it looks like you have multiple top-level headings in that file. The TOC code assumes a single page title, e.g.:

My Title
=====

Chapter 1
------------

Chapter 2
-----------

How would it work if there are multiple top-level headings?

As far as pygments, mkdocs-material supplies its own pygments style, and so I have disabled the normal pygments css. Did you custom pygments styles work?

Can you explain what the background color issue is? Also, can you reproduce those issues when you build the docs directly from the branch this pull request is based on? Because when I build the docs from this branch, the code blocks scroll horizontally, and tables have the data class.

jbms commented 2 years ago

Also, I'm thinking of following your suggestion and creating a fork for this version under a different name to avoid conflict with bashtage's package. I'm thinking of using the name sphinx-immaterial. Just using a number, like sphinx-material2, may be confusing if there is ever a version 2 of sphinx-material.

2bndy5 commented 2 years ago

How would it work if there are multiple top-level headings?

This examples.rst file's toc renders (on the right side of the page) like so: image Notice there's nothing after the second top-level heading. Using the master branch of this repo it renders like so image EDITED using pictures for (hopefully) better clarity

Did you custom pygments styles work?

Yes, it worked like a charm, but I'm willing to forgo that bit in the name of properly reviewing this.


sphinx-immaterial works just as well for me. This is really your hard-work I'm critiquing, so thanks for being patient with me. I just want to help get this modified theme deployed (very exciting changes).

2bndy5 commented 2 years ago

If you want to inspect the repo I'm using, I'm testing locally on my nRF24/CircuitPython_nRF24L01 repo (latest docs changes are hosted on RTFD.io). So, nothing regarding this modified theme has been commit.

jbms commented 2 years ago

Regarding the TOC issue, I see that in your index.rst you have a separate toctree with a caption of Examples that contains your examples page, and that it basically works with bashtage's existing theme.

However, I think Sphinx generally is designed around the assumption that there is a single top-level heading per page. For example, that is used as the title of the page; in your case, your examples document ends up with a title of the first heading, "nRF24L01 Features", but I would suppose that "Examples" would be a more suitable page title.

While it would probably be possible to fix my branch to support multiple top-level headings in some way, I think it would be simpler to just say that there has to be a single top-level heading. You could modify your document to be like:

Examples
=======

nRF24L01 Features
----------------------

Library-specific Features
---------------------------
2bndy5 commented 2 years ago

I'll look into a fix for your fork. This tactic I'm using worked for all other themes I tried (including RTD theme).

Yes, I should change the name of the top-level heading because of the material's theme-specific usage of the first heading. It renders fine in the navbar (with and without the toc.integrate feature).

I think Sphinx generally is designed around the assumption that there is a single top-level heading per page

I sincerly beg to differ.

  1. Sphinx is designed as an extension of docutils (or at least it started like that)
  2. It aims to adhere to all of the rst spec defined by docutils (& add more to it).
  3. If its not me, then I would expect someone else will eventually want to patch up multiple top-level headings.
jbms commented 2 years ago

Even in other themes, sphinx uses the first heading as the <title> of the page. Mkdocs-material additionally shows the first heading in the top bar once you scroll down past it.

Maybe it makes sense to support multiple top-level headings per page. It is just that the existing TOC logic is already quite complicated, and making it work with the mkdocs-material templates is also complicated because mkdocs has a somewhat different TOC model compared to Sphinx.

I will try to get the sphinx-immaterial repo set up tomorrow, and would certainly welcome your help as a maintainer.

jbms commented 2 years ago

I have set up the sphinx-immaterial repository: https://github.com/jbms/sphinx-immaterial

The documentation is mostly still unchanged from this repository and needs a lot of work, though.

Also the new autosummary used for the tensorstore documentation is not yet integrated, and the theme does not work that well with the normal autosummary. It also doesn't work that well yet with numpydoc (styling is a bit weird).

2bndy5 commented 2 years ago

This could probably be closed as it is now its own standalone theme. Unless, the intention is to attract attention to the sphinx-immaterial theme - this theme's landing page could use a mention/link for that though.