melissawm / sphinx-tags

A tiny Sphinx extension that implements blog-style tags for documentation.
https://sphinx-tags.readthedocs.io/en/latest/
MIT License
44 stars 11 forks source link

Add optional integration with sphinx-design badges #35

Closed JWCook closed 1 year ago

JWCook commented 1 year ago

Closes #9

Here are some changes for adding support for sphinx-design badges. Let me know what you think, or if you want to take a different approach.

Usage (settings in conf.py):

tags_create_badges = True

Optional support for specifying badge colors by tag name or glob pattern:

tags_badge_colors = {
    'coding': 'primary',
    'python': 'secondary',
    'prefix:*': 'warning',
    '*':  'light', 
}

Screenshot: image

melissawm commented 1 year ago

This is so cool - thank you so much! And yes - I am pretty sure this should go into 0.2 😄

The colors work great, but I am getting the following:

/home/melissa/projects/sphinx-tags/docs/dev/RELEASE.md:5: WARNING: 'any' reference target not found: tags/development
/home/melissa/projects/sphinx-tags/docs/examples.md:8: WARNING: 'any' reference target not found: tags/mdexamples
/home/melissa/projects/sphinx-tags/docs/examples.md:8: WARNING: 'any' reference target not found: tags/tagdocumentation
/home/melissa/projects/sphinx-tags/docs/quickstart.rst:6: WARNING: 'any' reference target not found: tags/tagdocumentation
/home/melissa/projects/sphinx-tags/docs/quickstart.rst:6: WARNING: 'any' reference target not found: tags/taginstallation

It looks like the class for the badge is not correctly detected, and is rendered as a "xref any" which doesn't create a proper link.

<p class="tags"><span>Tags in this page: </span><span class="xref any sd-sphinx-override sd-badge">tagdocumentation</span><span> </span><span class="xref any sd-sphinx-override sd-badge">taginstallation</span></p>

I can try and investigate a bit further but if you have ideas that would be great. Cheers!

JWCook commented 1 year ago

That's odd. I couldn't reproduce this on my Linux machine, and badge links get generated like this:

<a class="sd-sphinx-override sd-badge sd-bg-primary sd-bg-text-primary reference internal" href="tags/python.html">
  <span class="doc">python</span>
</a>

But I was able to reproduce this on Windows. I'll look into it some more later this week.

JWCook commented 1 year ago

I'm still a bit stuck on this, but was at least able to eliminate simple dependency mismatches as a cause. I have a Windows virtualenv and a Linux virtualenv with the exact same dependencies and versions, and the problem still only occurs on Windows (tested on two different Windows computers).

It seems that the <output_dir>/<tagname> page doesn't get generated, so there's nothing to link to. The any class added to the badge may or may not be a red herring. Weird!

JWCook commented 1 year ago

Oof. Some of the tags I was using to test were formatted like status:draft... which happens to be a valid filename in Linux but not Windows. So that was a completely different problem. I can make a separate PR to normalize tag filenames to avoid that.

JWCook commented 1 year ago

@melissawm Alright, I found the problem. I was just passing the wrong reference path to XRefBadgeRole. Badges now render correctly in the docs for this repo:

image

melissawm commented 1 year ago

This is awesome! I will double check later today (on mobile now) but as soon as I merge this and #37 I will cut a 0.2 release with these new features. Thanks again!

melissawm commented 1 year ago

Ok! Because of #37 there are now conflicts here - do you want to try and fix them @JWCook , or should I do it?

melissawm commented 1 year ago

Testing locally it looks like the conflict can be solved if run is as follows:

    def run(self):
        tags = [arg.replace(self.separator, "") for arg in self.arguments]
        tag_dir = Path(self.env.app.srcdir) / self.env.app.config.tags_output_dir
        result = nodes.paragraph()
        result["classes"] = ["tags"]
        result += nodes.inline(text=f"{self.env.app.config.tags_intro_text} ")
        count = 0

        for tag in tags:
            count += 1
            # We want the link to be the path to the _tags folder, relative to
            # this document's path where
            #
            #  - self.env.app.config.tags_output_dir
            # |
            #  - subfolder
            #   |
            #    - current_doc_dir
            current_doc_dir = Path(self.env.doc2path(self.env.docname)).parent
            relative_tag_dir = os.path.relpath(tag_dir, current_doc_dir)

            if self.env.app.config.tags_create_badges:
                result += self._get_badge_node(tag, relative_tag_dir)
                tag_separator = " "
            else:
                result += self._get_plaintext_node(tag, relative_tag_dir)
                tag_separator = f"{self.separator} "
            if not count == len(tags):
                result += nodes.inline(text=tag_separator)
        return [result]
JWCook commented 1 year ago

Alright, fixed and rebased!

melissawm commented 1 year ago

Well, something is wrong but I'm not really sure what 🤔 I had tested it all locally and all was ok... I tried setting the python version (I'm having trouble in other projects with Python 3.11) but that was not it.

JWCook commented 1 year ago

Looks like it has something to do with passing the docutils Inliner object (which handles inline markup) to sphinx_design.badges_buttons.XRefBadgeRole. That happens here: https://github.com/melissawm/sphinx-tags/pull/35/files#diff-bdc0bd8928b62396fa2f71fdca2862a25f43b7236a6f59c0663d479c2b481669R85

Although I don't see anything obvious that changed between the last working build and this one.

I'll look into it some more.

JWCook commented 1 year ago

Aha! It's a bug exposed by a combination of this PR and the previous one. Specifically, this line and this line.

It seems that if a tags role is the first non-blank line of a document, then update_tags() gets called before some docutils internals are initialized, including SphinxDirective.state.inliner.document. No errors occurred immediately after merging #41 since nothing was touching that attribute yet.

This probably means that now that we're passing some extra state to sphinx-design that was previously unused, this extension should be connected to a different event later in the build process, although I'm not familiar enough with the whole event system to know where exactly it should go.

melissawm commented 1 year ago

oof! Thanks for the detective work - I will come back to investigate later. Cheers!

melissawm commented 1 year ago

I am going to merge this and figure out the bug in a bit. Thank you so much and hope to see you around! :tada:

JWCook commented 1 year ago

Thanks! I'll also try to look into this a bit more.

I think the connected event doesn't need to change after all, it seems specific to parsing rST. If you put tags at the top of a MyST document, it works fine. Currently digging through docutils.parsers.rst source code to figure out where this is supposed to be initialized.