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

Add support for Markdown extension options. #435

Merged
merged 2 commits into from Apr 15, 2015

Conversation

Projects
None yet
4 participants
@waylan
Member

waylan commented Apr 8, 2015

I tried to keep the docs simple so non-python people would understand them
(I avoided calling the setting a 'dict,' etc.). Could probably use some
improvement.

All this extension mungling stuff should probably go in validate_config,
but that feels like an entireley differant issue than this, so I made the
changes inplace. Fixes #351

waylan added some commits Apr 8, 2015

Add support for Markdown extension options.
I tried to keep the docs simple so non-python people would understand them
(I avoided calling the setting a 'dict,' etc.). Could probably use some
improvement.

All this extension mungling stuff should probably go in validate_config,
but that feels like an entireley differant issue than this, so I made the
changes inplace. Fixes #351

@d0ugal d0ugal added this to the 0.13.0 milestone Apr 8, 2015

@d0ugal d0ugal added the Enhancement label Apr 8, 2015

@d0ugal

This comment has been minimized.

Member

d0ugal commented Apr 8, 2015

Looks good. Going to push this to 0.13, just so I can get 0.12 out the door ASAP, it is long overdue.

d0ugal added a commit that referenced this pull request Apr 15, 2015

Merge pull request #435 from waylan/mdx_config
Add support for Markdown extension options.

@d0ugal d0ugal merged commit e7aef7a into mkdocs:master Apr 15, 2015

3 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.17%) to 79.15%
Details
@d0ugal

This comment has been minimized.

Member

d0ugal commented Apr 15, 2015

Thanks!

@fedelibre

This comment has been minimized.

Contributor

fedelibre commented Apr 15, 2015

@d0ugal How did you merge it?
In master with git log -p I can see only the merge commit but not the real commits

@d0ugal

This comment has been minimized.

Member

d0ugal commented Apr 15, 2015

@fedelibre They are there, you just need to go down far enough as they are 7 days old.

@waylan waylan deleted the waylan:mdx_config branch Apr 15, 2015

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 26, 2015

I am curious about this implementation. I noticed that the new way to define configs for an extension went from an ordered list to an unordered dictionary.

Now I understand that all of Python Markdown's default extensions are not order dependent only because the number of extensions is limited, but with 3rd party extensions, every new extension makes things....more complicated. I have written a number of Python Markdown extension for personal use, and though I have found ways to workaround these issues, it is still based on the limited extension set that I use.

So, let's say you have 3 extensions that all need to be the first preprocessor. Order of inclusion will determine who actually gets it; they will all be at the beginning, but only one can actually be first. Personally, I think keeping the array (as shown below) would allow a user to work around such issues. If they don't play nice in any order, at least the user can still control the order to fix the problem. I am just trying to be forward thinking:

markdown_extensions:
- some_extensions: 
- some_otherextension:
     config_item: 1

Any thoughts?

@d0ugal

This comment has been minimized.

Member

d0ugal commented Apr 26, 2015

Good question. I'll defer to @waylan, as our resident Python-markdown expert.

One thing tho' - I have a feeling pyYAML uses an OrderedDict but I've not checked this.

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 26, 2015

One thing tho' - I have a feeling pyYAML uses an OrderedDict but I've not checked this.

That would be good. I'll have to double check that.

If it is ordered though, we would lose it all here as sets are not ordered:

extensions = set(builtin_extensions + mkdocs_extensions + user_extensions)
@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 26, 2015

Looks like PyYAML dicts are not ordered 😦 :

>>> print(yaml.load("""
... test:
...     apple: 1
...     orange: 2
...     bananna: 3
... """))
{'test': {'orange': 2, 'bananna': 3, 'apple': 1}}
@waylan

This comment has been minimized.

Member

waylan commented Apr 27, 2015

This requires some customization of the YAML parser. See here. I haven't taken a close look at each of the options listed yet, but suspect at least one of them could be used.

There are a number of solutions listed for ordered sets here. Again, I haven't taken a close look yet.

The point is, these are solvable problems. And I think this is more desirable than creating another config setting. Why have two settings for extensions (a list of extension names and a dict of config options) when it can all be represented in one?

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 27, 2015

I have no problem working a solution around the current setting, but was more pointing out the issue before it gets released. I have overridden parts of pyyaml before, so that isn't a problem. And I don't think we need an ordered set as we can accomplish the same thing with a little code to remove duplicates.

I can probably look into issuing a pull request.

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 27, 2015

Looks to be pretty easy, so if you need ordered, this solution would just require you to use !odict, and use the new method to reduce the list, order is preserved there at all times as well:

import yaml
from collections import OrderedDict


def odict_constructor(loader, node):
    """
    Create !odict for ordered dictionaries
    markdown_extensions: !odict
        toc:
        admonition:
    """
    return OrderedDict(loader.construct_pairs(node))

yaml.add_constructor(u'!odict', odict_constructor)


def reduce_list(extensions):
    """ Reduce duplicate items in a list and preserve order """
    seen = set()
    return [ext for ext in extensions if ext not in seen and not seen.add(ext)]


text = '''
test: !odict
    key1: 1
    key2: 2
    key3: 3
    key4: 4
    key5: 5
    key5: 6
    key3: 7
'''

obj = yaml.load(text)

print('Dict is ordered')
print(obj)
print('List is reduced with order')
print(reduce_list(obj['test']))

Output:

Dict is ordered
{'test': OrderedDict([('key1', 1), ('key2', 2), ('key3', 7), ('key4', 4), ('key5', 6)])}
List is reduced with order
['key1', 'key2', 'key3', 'key4', 'key5']

If this looks okay, I can issue a pull request.

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 27, 2015

I guess this wouldn't work on py26 though, but it looks like you are looking to drop py26 support anyways. I guess until then, the YAML OrderedDict part could be excluded for py26.

So really the only that changes here is list order would be preserved when reducing duplicate extensions. And only if the user specifies !odict would a dict be ordered, otherwise it would be unordered.

@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 27, 2015

I guess the other options would be to order all dictionaries (no !odict needed). Output would be the same as above:

import yaml
from collections import OrderedDict


# def odict_constructor(loader, node):
#     """
#     Create !odict for ordered dictionaries
#     markdown_extensions: !odict
#         toc:
#         admonition:
#     """
#     return OrderedDict(loader.construct_pairs(node))

# yaml.add_constructor(u'!odict', odict_constructor)


_mapping_tag = yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG


def dict_representer(dumper, data):
    return dumper.represent_dict(data.iteritems())


def dict_constructor(loader, node):
    return OrderedDict(loader.construct_pairs(node))

yaml.add_representer(OrderedDict, dict_representer)
yaml.add_constructor(_mapping_tag, dict_constructor)


def reduce_list(extensions):
    """ Reduce duplicate items in a list and preserve order """
    seen = set()
    return [ext for ext in extensions if ext not in seen and not seen.add(ext)]


text = '''
test:
    key1: 1
    key2: 2
    key3: 3
    key4: 4
    key5: 5
    key5: 6
    key3: 7
'''

obj = yaml.load(text)

print('Dict is ordered')
print(obj)
print('List is reduced with order')
print(reduce_list(obj['test']))
@facelessuser

This comment has been minimized.

Contributor

facelessuser commented Apr 27, 2015

Much better implementation as it doesn't modify the PyYaml object. This is preferable if mkdocs is ever exposed as a module; we wouldn't want it to affect the PyYaml default output globally. I think this is the way I will go.

import yaml
from collections import OrderedDict

__all__ = ["reduce_list", "yaml_load"]


def yaml_load(stream, Loader=yaml.Loader, object_pairs_hook=OrderedDict):
    """
    Make all YAML dictionaries load as ordered Dicts.
    http://stackoverflow.com/a/21912744/3609487
    """
    class OrderedLoader(Loader):
        pass

    def construct_mapping(loader, node):
        loader.flatten_mapping(node)
        return object_pairs_hook(loader.construct_pairs(node))

    OrderedLoader.add_constructor(
        yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
        construct_mapping
    )

    return yaml.load(stream, OrderedLoader)


def reduce_list(data_set):
    """ Reduce duplicate items in a list and preserve order """
    seen = set()
    return [item for item in data_set if item not in seen and not seen.add(item)]


if __name__ == "__main__":
    text = '''
test:
    key1: 1
    key2: 2
    key3: 3
    key4: 4
    key5: 5
    key5: 6
    key3: 7
'''

    obj = yaml_load(text)

    print('Dict is ordered')
    print(obj)
    print('List is reduced with order')
    print(reduce_list([1, 2, 3, 4, 5, 5, 2, 4, 6, 7, 8]))

output

Dict is ordered
OrderedDict([('test', OrderedDict([('key1', 1), ('key2', 2), ('key3', 7), ('key4', 4), ('key5', 6)]))])
List is reduced with order
[1, 2, 3, 4, 5, 6, 7, 8]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment