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

Improve error message for missing Markdown extensions #782

Closed
sils opened this Issue Dec 30, 2015 · 9 comments

Comments

Projects
None yet
4 participants
@sils

sils commented Dec 30, 2015

I just had to install that manually.

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Dec 30, 2015

Pymdown-extensions is not a required set of extensions as Mkdocs works great without them. Those extensions are just extras you can install if you like one or more of the extensions. I use them, but that is also because I wrote them :).

@sils

This comment has been minimized.

sils commented Dec 30, 2015

Ah, because I use pymdownx.superfences explicitly! Maybe you could catch the import error and promt the user nicely for installing pymdown-extensions?

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Dec 30, 2015

Yeah, that is really a Python Markdown thing. It is set to fail on extension import error.

@d0ugal d0ugal added the Enhancement label Jan 28, 2016

@d0ugal

This comment has been minimized.

Member

d0ugal commented Jan 28, 2016

Yeah, okay - let's show a better error here if we can. Even if it just a nicer error that states something like this: "Your mkdocs.yml has a python-markdown extension that you don't seem to have installed locally"

It is surprisingly difficult for us to tell users what package they need to install, because: Python packaging 😄

@waylan

This comment has been minimized.

Member

waylan commented Jan 28, 2016

Right. You are wrapping a library, which raises an ImportError when given a bad (or uninstalled) extension name. The tricky part is that MkDocs does not preload all of the extensions, but only passes the list of extension names on to Markdown on page build (and this happens every time for every single page built). I see three options:

  1. Wrap utils.convert_markdown in a try/except block and on ImportError stop the build process returning a friendly error. The error would occur on the first page being built. This could result in a "dirty" partially built site_dir.
  2. Have Mkdocs load the extensions itself (as part of the config validation) and raise errors then. Markdown will accept instances of extensions in addition to string names, so then just pass the instances to Markdown. Of course, this replicates some of the code already in Markdown and could be subject to change in future releases.
  3. Create a throwaway instance of the Markdown class in the config validation step and have validation fail on error. It has to be a throwaway instance because each individual page needs its own custom version of the internal (to MkDocs) RealativePathExtension (see mkdocs/commands/build.py#L28). Therefore, MkDocs can't just create one Markdown instance and reuse it for every page. To be clear, I would expect the throwaway instance to not include the RealativePathExtension. We simply need to validate that all user provided extension names are loadable/importable.

My suggestion would be 1 or 3 above. There is no mechanism in place within MkDocs to handle an error raised during build. All the error handling is relegated to the config validation, which is why I mention options 2 and 3 (not to mention the potential for a "dirty" site dir in option 1). Option 3 sees like the best fix to me.

@waylan

This comment has been minimized.

Member

waylan commented Jan 28, 2016

It is surprisingly difficult for us to tell users what package they need to install, because: Python packaging

Also because many times the user simply spelled it wrong, in which case nothing needs to be installed. However, it is nearly impossible to tell which it is and writing a "friendly" error message which covers both is tricky.

@waylan

This comment has been minimized.

Member

waylan commented Jan 28, 2016

A potential problem just occurred to me for both options 1 and 3 above. The error message provided by Markdown will need to be parsed to get the specific extension name which raised the error. You could just pass the message on, but that may not be "friendly".

However, I don't recommend option 2 as the upcoming version 3 of the Markdown lib will be switching to using SetupTools entry points to load extensions by name. An option 2 solution will not work across Python-Markdown versions 2.x and 3.x.

@d0ugal

This comment has been minimized.

Member

d0ugal commented Jan 28, 2016

switching to using SetupTools entry points to load extensions by name

Neat!

@waylan waylan added this to the 1.0.0 milestone Dec 2, 2016

@waylan waylan changed the title from Missing dependency: pymdown-extensions to Improve error message for missing Markdown extensions Apr 4, 2018

@waylan

This comment has been minimized.

Member

waylan commented Apr 6, 2018

I have a working solution for option 3 above. If an extension foo is listed in the config, I get this output:

$ mkdocs build
ERROR   -  Config value: 'markdown_extensions'. Error: Failed loading extension 'foo' from 'foo', 'markdown.extensions.foo' or 'mdx_foo'

Aborted with 1 Configuration Errors!

The final part at the end of the message (after Error: ) is directly from Markdown's error message. We don't tell the user to "install" the extension because we don't know if the extension isn't installed or if they spelled it wrong. The important thing is that it's clear that the problem is related to the markdown_extensions config value foo. The user now understands that they need to work out why the Markdown extension foo does't work. Connecting the error to the markdown_extensions config would not have been possible with option 1, but we get it for free with option 3.

waylan added a commit to waylan/mkdocs that referenced this issue Apr 6, 2018

Improve Markdown extention error messages.
Fixes mkdocs#782. Note that we mock Markdown in the tests to ensure those
tests are not using Markdown to validate the extension names. We don't
mock Markdown in the tests which we do want Markdown to validate the
extension names.

waylan added a commit to waylan/mkdocs that referenced this issue Apr 6, 2018

Improve Markdown extension error messages.
Fixes mkdocs#782. Note that we mock Markdown in the tests to ensure those
tests are not using Markdown to validate the extension names. We don't
mock Markdown in the tests which we do want Markdown to validate the
extension names.

waylan added a commit to waylan/mkdocs that referenced this issue Apr 6, 2018

Improve Markdown extension error messages.
Fixes mkdocs#782. Note that we mock Markdown in the tests to ensure those
tests are not using Markdown to validate the extension names. We don't
mock Markdown in the tests which we do want Markdown to validate the
extension names.

waylan added a commit to waylan/mkdocs that referenced this issue Apr 6, 2018

Improve Markdown extension error messages.
Fixes mkdocs#782. Note that we mock Markdown in the tests to ensure those
tests are not using Markdown to validate the extension names. We don't
mock Markdown in the tests which we do want Markdown to validate the
extension names.

Also ensure project-min has all relevant extensions.

@waylan waylan closed this in #1459 Apr 6, 2018

waylan added a commit that referenced this issue Apr 6, 2018

Improve Markdown extension error messages.
Fixes #782. Note that we mock Markdown in the tests to ensure those
tests are not using Markdown to validate the extension names. We don't
mock Markdown in the tests which we do want Markdown to validate the
extension names.

Also ensure project-min has all relevant extensions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment