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

fix #78 (ImportError: cannot import name make_admonition in Sphinx 1.6) #79

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lsaffre
Copy link
Contributor

@lsaffre lsaffre commented Feb 5, 2017

This fixes #78 for me.

@lsaffre
Copy link
Contributor Author

lsaffre commented Mar 10, 2017

Hi Ahmet, what do you think about my pull request? Did you see it at all?

@abakan-zz
Copy link
Owner

Where can I find Sphinx 1.6? I didn't see it in usual places.

ablog/post.py Outdated
from sphinx.util.compat import Directive, make_admonition
from sphinx.util.compat import Directive
# DeprecationWarning: make_admonition is deprecated, use docutils.parsers.rst.directives.admonitions.BaseAdmonition instead
from docutils.parsers.rst.directives.admonitions import BaseAdmonition as make_admonition
Copy link
Owner

@abakan-zz abakan-zz Mar 12, 2017

Choose a reason for hiding this comment

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

Above should be like

try:
    from sphinx.util.compat import make_admonition		
except ImportError:
    from docutils.parsers.rst.directives.admonitions import BaseAdmonition as make_admonition

so that this still works with older versions of Sphinx.

ablog/post.py Outdated




Copy link
Owner

Choose a reason for hiding this comment

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

These extra lines needs to be removed.

@lsaffre
Copy link
Contributor Author

lsaffre commented Mar 13, 2017

@abakan, thanks for your explanations. I adapted my PR accordingly. I see that Travis build is still failing, but I have the impression that these failures are not caused by my changes (are they?)

@abakan-zz
Copy link
Owner

abakan-zz commented Mar 13, 2017 via email

@lsaffre
Copy link
Contributor Author

lsaffre commented Mar 13, 2017

Oops, you are right. I saw that it was not being used in that module, which was of course a naive assumption. I now restored that line.

@timcera
Copy link

timcera commented May 25, 2017

Thanks for this fix. Solved my problem.

readmodifywrite added a commit to readmodifywrite/ablog that referenced this pull request Jul 21, 2017
@rayalan
Copy link

rayalan commented Dec 13, 2017

Still works with Sphinx 1.6.5 / Python 3.6. Would be great to see this integrated into a release.

@rayalan
Copy link

rayalan commented Jan 21, 2018

Note that this fix causes the .. update directive to no longer print the title because of the way docutil handles the BaseAdmonition class. Here's a partial patch that restores similar functionality (slight change to the formatting, doesn't do date verification any more)

class UpdateDirective(BaseAdmonition):
    required_arguments = 1
    node_class = UpdateNode

    def run(self):
        date_arg = self.arguments[0] if self.arguments else ''
        self.arguments = [_('Updated on ' + date_arg), ]
        # The following line is needed to trick the BaseAdmonition class into thinking we have a title.
        # There almost certainly has to be a better way, but I don't see it naturally from the docutils source    
        self.node_class = nodes.admonition
        ad = super(UpdateDirective, self).run()  # For Python3, it is run(), not run(self)
        del self.node_class  # Undo our tricksy hack from a few lines ago
        return ad

@nabobalis
Copy link

This has now been merged into here.
Thank you for the PR!

Repository owner deleted a comment from DavideStagni Mar 19, 2024
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.

5 participants