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

Rebase on new version of mkdocs-material #96

Closed
wants to merge 1 commit into from
Closed

Conversation

jbms
Copy link

@jbms jbms commented Mar 2, 2021

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

  • CSS and Javascript bundle are generated from the original SCSS and Typescript sources derived from mkdocs-material.
  • The bs4-based munging of section headings, tables, and the table of contents has been replaced by cleaner hooks.
  • As mkdocs-material is very complex, I have attempted to minimize the deviation from it in order to make it easier to stay in sync with it in the future. In general, I think it is valuable to be able to stay up to date with mkdocs-material since it has very heavy development, but that means it is likely not practical to support significant deviation from it. In particular, I have attempted to have the Typescript/CSS/HTML file paths exactly match the mkdocs-material repository, in order to simplify rebasing.
  • Support for textual "heroes" was eliminated some time ago from mkdocs-material, but I patched that back in since it seems like a nice feature. However, this deviation does seem worrisome from a maintenance perspective and it does seem possible that in the future it may prove difficult to keep.
  • Support for breadcrumbs and other nav links in the header has been eliminated since that is not supported by mkdocs-material.
  • All of the mkdocs-material feature flags work, including "instant navigation". Note that instant navigation in mkdocs-material is currently based on the xml sitemap, which means it only works if the correct base_url is specified when building.
  • I changed the versions.json format to match the "mike" format supported by mkdocs-material.
  • Table alignment now works, and table classes are no longer stripped.
  • Dark mode works, including automatic selection based on user preference.
  • I changed the theme configuration options a bit for consistency with mkdocs-material.
  • mkdocs-material includes ~30MB of icons --- that seemed like unnecessary bloat, so I instead set things up to only include the icons actually used. That does mean if users want to include icons they will have to arrange to include them, though. That also means the logo icon support has been removed as well. It seems like the logo icons are unlikely to be used except as placeholders anyway, though.
  • On top of the normal sphinx search index, I implemented search-as-you-type functionality similar to that found in mkdocs-material, which hooks into the search mkdocs-material search UI. The differences from mkdocs-material are as follows:
    • mkdocs uses lunr for searching, which requires loading a rather large search index json file, and if the "prebuilt index" mode is used, it can be significantly larger even than the full text content of the website.
    • mkdocs-material shows results for individual sections of a page, but just shows the start of the section as a snippet.
    • sphinx uses a custom search mechanism that uses a significantly smaller index, because it does not contain the full text itself. In order to show snippets after finding the results, sphinx fetches the actual target html document, extracts the main content and finds the first occurrence of a search term. If it fails to fetch the document (e.g. due to being run from a file:// URL) then it just doesn't show a snippet.
    • sphinx includes a special search index for "objects", e.g. classes, methods, etc.
    • sphinx normally provides links only to top-level documents, not individual sections, however I have implemented section links as well as part of the modified snippet extraction.
    • Since extracting the section titles and snippets requires retrieving the actual target documents, for a large number of results this could take a while. To mitigate that, fetching is controlled by the scroll position of the results view --- snippets are only fetched for visible results, and then more are fetched if the user scrolls down.

TODO:

  • "Productions" are currently broken.
  • I removed the --page-root munging because I couldn't figure out what it was for. However, that might need to be added back if it is still necessary.
  • Documentation regarding theme options needs to be updated.
  • Function/class documentation styling needs to be fixed.

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

@pep8speaks
Copy link

pep8speaks commented Mar 2, 2021

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
Copy link

ghost commented Mar 2, 2021

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
Copy link
Author

jbms commented Mar 2, 2021

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
Copy link
Author

jbms commented Mar 2, 2021

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.

@jbms jbms force-pushed the master branch 3 times, most recently from f7393f5 to eadcf67 Compare March 3, 2021 00:28
@jbms jbms mentioned this pull request Mar 3, 2021
@bashtage
Copy link
Owner

bashtage commented Mar 5, 2021

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
Copy link
Author

jbms commented Mar 5, 2021

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
Copy link

pradyunsg commented Mar 31, 2021

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
Copy link
Author

jbms commented Mar 31, 2021

@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
Copy link

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
Copy link
Author

jbms commented Apr 1, 2021 via email

@nejch
Copy link

nejch commented May 1, 2021

@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
Copy link
Author

jbms commented Aug 1, 2021

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
Copy link

swarnat commented Aug 4, 2021

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
Copy link

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
Copy link

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
Copy link
Owner

bashtage commented Aug 4, 2021

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
Copy link
Owner

bashtage commented Aug 4, 2021

@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
Copy link
Author

jbms commented Aug 4, 2021

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 sphinx-doc/sphinx#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
Copy link

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
Copy link

@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. 🙏

raise RuntimeError('Could not run \'npm run %s\'.' % t)

if res != 0:
raise RuntimeError('failed to build sphinx-material package')
Copy link

@2bndy5 2bndy5 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to integrate this modified branch into my project's docs/requirements.txt like so

git+https://github.com/jbms/sphinx-material
sphinx-copybutton

But running python -m pip install -r docs/requirements.txt fails to build the sphinx-material theme. It's traceback ends on this line.


Furthermore, I did a fresh install of npm and retried... It failed again, but this time, npm reported

Error: Cannot find module 'fs/promises'
      at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
      at Function.Module._load (internal/modules/cjs/loader.js:562:25)
      at Module.require (internal/modules/cjs/loader.js:692:17)
      at require (internal/modules/cjs/helpers.js:25:18)
      at Object.<anonymous> (/tmp/pip-req-build-7y539i6_/tools/build/_/index.ts:24:1)
      at Module._compile (internal/modules/cjs/loader.js:778:30)
      at Module.m._compile (/tmp/pip-req-build-7y539i6_/node_modules/ts-node/src/index.ts:1056:23)
      at Module._extensions..js (internal/modules/cjs/loader.js:789:10)
      at Object.require.extensions.(anonymous function) [as .ts] (/tmp/pip-req-build-7y539i6_/node_modules/ts-node/src/index.ts:1059:12)
      at Module.load (internal/modules/cjs/loader.js:653:32)

Is there a required npm (non-std) lib also? I did find this commit comment to be helpful, but I'm not well versed with node.js.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may just need to update your version of nodejs.

Copy link

@2bndy5 2bndy5 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nodejs -v returns v10.19.0. I'll have to look into how to update that. I installed npm via (WSL) ubuntu apt, so I might find more problems from that.

I wondor if this requirement might break compatibility with readthedocs.org... I wouldn't think so since it can host docs generated with mkdocs-material.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could look into it more, I was just guessing, but I was testing with nodejs version 14 I believe.

mkdocs-material does not require users of the package to have node.js because the author re-computes the generated files and checks them into the repository after every change.

That has a number of drawbacks. What I have setup instead in my branch is that if you build from the repository you will need nodejs because the generated file will be rebuilt automatically, but when you build a wheel (e.g. for publishing on pypi) the generated files will be pre-built. So users will still be able to pip install without needing nodejs.

Copy link

@2bndy5 2bndy5 Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I was able to update nodejs (from v10 to v16) via nvm, but I had to uninstall npm and nodejs via ubuntu apt first (it was problematic as I expected). I'm now able to install the the modified theme. Thank you for the assist!

I looked into readthedocs... Their docker image source seems to install npm and nodejs via apt 😦 which means this modified theme needs to be installed via pypi to avoid needing nodejs v14+. However, the RTFD manual suggests using a config file to ensure other software requirements are met, but I doubt that would help my situation (installing modified theme from git not pypi) due to the conflicts that installing nodejs from apt incur.

@2bndy5
Copy link

2bndy5 commented Sep 24, 2021

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
Copy link
Author

jbms commented Sep 25, 2021

@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
Copy link

2bndy5 commented Sep 25, 2021

@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
Copy link
Author

jbms commented Sep 25, 2021

I just pushed an update that fixes that problem.

@2bndy5
Copy link

2bndy5 commented Sep 25, 2021

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

@2bndy5
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

  • code-blocks don't scroll horizontally (a must for mobile displays)
    • This is consistent with tensorstore site as well.
      adding the following rule fixed this, but the line numbers scroll with the code (not a huge concern for me).
      .highlight pre {
        overflow-x: auto;
      }
  • table's (and table cell's) border aren't visible
    • comparing my results with the tensorstore docs, tensostore doc's tables use the following css
      /* copied from firefox inspection:
         naming _typeset.scss:493 as the source */
      .md-typeset table.data:not(.plain) {
          background-color: var(--md-default-bg-color);
          border-radius: .1rem;
          box-shadow: 0 .2rem .5rem rgba(0,0,0,.05),0 0 .05rem rgba(0,0,0,.1);
          display: table;
          font-size: .64rem;
          max-width: 100%;
          overflow: auto;
          touch-action: auto;
      }
      But the tables for my generated docs don't have the class named data assigned to them
      my docs' tables' classes tensorstore's tables' classes
      colwidths-given docutils align-default colwidths-auto docutils data align-default

@jbms
Copy link
Author

jbms commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link
Author

jbms commented Sep 25, 2021

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
Copy link
Author

jbms commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link
Author

jbms commented Sep 25, 2021

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
Copy link

2bndy5 commented Sep 25, 2021

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
Copy link
Author

jbms commented Sep 25, 2021

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
Copy link
Author

jbms commented Sep 30, 2021

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
Copy link

2bndy5 commented Apr 12, 2022

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.

@jbms jbms closed this Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants