Avoid Markdown 2.6 deprecations #1638

kernc · 2015-02-23T22:31:10Z

Short extension names ('extra', 'meta') are deprecated
https://pythonhosted.org/Markdown/release-2.6.html#shortened-extension-names-deprecated
Extension config as part of extension name is deprecated
https://pythonhosted.org/Markdown/release-2.6.html#extension-configuration-as-part-of-extension-name-deprecated

The MD_EXTENSIONS documentation in docs/settings.rst hasn't been touched, so it still shows the old value.

kernc · 2015-02-23T22:41:13Z

pelican/readers.py

+                'css_class':'highlight'
+            }
+        self._md = Markdown(extensions=self.extensions,
+                            extension_configs=extension_configs)


I don't think this will surprise too many unweary users. If they want some other css_class, they will have had something else in MD_EXTENSIONS anyway.

ingwinlu · 2015-02-24T06:02:57Z

Hey,

gonna pile on this: wasn't headerid part of extra before it now got deprecated? I don't think we run tests on that currently, but the behaviour of markdownreader will change through an update (i.e. header ids will not be available anymore)

About the config thing, Can we not just blindly overwrite or correct settings? Would be better to use our deprecation functions somehow and warn the user that we are changing his settings to the now correct ones.

ingwinlu · 2015-02-24T06:11:46Z

Apparently I was wrong and extra never included headerid

kernc · 2015-02-25T01:22:41Z

What about a code like this:

settings.py

'MD_EXTENSIONS': [
    'codehilite-INTERNAL-ONLY',  # something that would never have been entered by a user
    'markdown.extensions.extra'
],

readers.py

if 'codehilite-INTERNAL-ONLY' in self.extensions:
    ... # correct the plugin name
    ... # set the default css_class setting as above

I wouldn't know how to use the deprecation machinery here, nor how it applies.

avaris · 2015-02-25T02:52:59Z

It's probably better to expose extension_configs as a setting since any string extension has to be configured with that. It'll also make setting YAML meta easier:

'MD_EXTENSIONS': [
    'markdown.extensions.codehilite'
    'markdown.extensions.extra'
],
'MD_EXTENSION_CONFIGS' : {'markdown.extensions.codehilite' : {'css_class': 'highlight' }},

and the user defined config will .update the default one, so people can do:

MD_EXTENSION_CONFIGS = {'markdown.extensions.meta': {'yaml': True}}

to turn on YAML processing.

Besides that, settings.py has to correct old MD_EXTENSIONS settings defined by the user (and also warn them) to the new one. If correcting becomes too tricky, we can just warn that the syntax is changed and fall back to default values.

kernc · 2015-02-27T00:47:19Z

~~So then when Markdown decides to break its API with 3.0, wrap everything once again to provide backwards compatibility? I'm against it. The MD_EXTENSIONS interface is clear: If you need special config parameters, you can instantiate the plugin class directly as you want it.~~Ok, that could work.

kernc · 2015-02-28T03:37:21Z

docs/settings.rst

@@ -39,184 +39,186 @@ Here is a list of settings for Pelican:
 Basic settings
 ==============

-===============================================================================  =====================================================================
-Setting name (followed by default value, if any)                                 What does it do?
-===============================================================================  =====================================================================


git diff --ignore-space-change shows the true changes here.

kernc · 2015-02-28T03:56:41Z

How about now? 😅

justinmayer · 2015-05-02T16:22:14Z

@avaris: Thoughts on @kernc's latest?

avaris · 2015-05-02T20:32:43Z

@justinmayer Ah, sorry, I somehow missed the last commit.

@kernc Looks mostly OK. Use of eval is a bit troublesome. I can see why it was used but still it would be best to avoid.

EDIT: On second look, that eval won't even work. Consider the old (corrected) default markdown.extensions.codehilite(css_class=highlight): It'd try to do eval('dict(css_class=highlight)') which should result in NameError for highlight.

ingwinlu · 2015-05-03T08:11:23Z

Am I the only one who is against introducing setting deprectiation in readers.py?

ingwinlu · 2015-08-18T11:57:59Z

@kernc any update on this? Can you move the depreciation into the appropriate section?

@justinmayer might want to tag this to keep track of it for soonish release

justinmayer · 2015-08-19T19:51:53Z

@ingwinlu: Tagged. Thanks!

kernc · 2015-08-19T22:14:17Z

Have considered both @avaris' and @ingwinlu's recommendations. Particularly the provided link was really helpful to a project novice like myself. That dict eval-ing is kind of hackish, but good enough and should work in all practical cases, I think.

ingwinlu · 2015-08-19T22:33:44Z

hey @kernc good to see you are still on board with this.

some food for thought:

can we reduce the configs and replace them with a single dict? we can iterate over that dict's keys to get the names identifying the extensions
can we somehow get the eval out? either replace it with a regex or abort when the config is not correct and let users handle it manually

there also appears quite some ws in this commit, is that really necessary (in the doc)?

kernc · 2015-08-24T16:02:01Z

@ingwinlu I agree with your considerations, will implement asap.

Re table whitespace: the default value string should get pretty long. Should I put the default value in the right-hand column instead?

justinmayer · 2015-09-02T00:17:57Z

@kernc: I think putting the default value in the right-hand column for this particular setting might indeed make sense, obviating the need to adjust the spacing for the entire table.

justinmayer · 2015-11-05T20:40:27Z

Hi @kernc. Just following up on this PR. Any chance you might be able to get it ready for merge as noted above?

kernc · 2015-11-08T22:33:05Z

@justinmayer sorry, thanks. 😳

@ingwinlu How would I "reduce the configs and replace them with a single dict"? Isn't that exactly the opposite of what @avaris recommended?

I also did (almost) drop the eval? Is it better now or is it still unsafe?

As proposed by in getpelican#1638 (comment)

ingwinlu · 2015-11-15T13:32:16Z

docs/internals.rst

@@ -50,7 +50,8 @@ Take a look at the Markdown reader::
        def read(self, source_path):
            """Parse content and metadata of markdown files"""
            text = pelican_open(source_path)
-            md = Markdown(extensions = ['meta', 'codehilite'])


needs to be updated as well

Thanks for catching that. 😅

As proposed by in getpelican#1638 (comment)

avaris · 2015-11-15T14:57:51Z

Why are we hard-quitting after a misconfiguration that doesn't have any major effect? I seriously disagree with that. It should be treated same as any other misconfiguration: ignore and fall back to default with a warning.

As proposed by in getpelican#1638

kernc · 2015-11-15T19:05:40Z

That's exactly why I asked, thanks.

ingwinlu · 2015-11-29T08:41:13Z

pelican/settings.py

+        logger.warning('The format of the MD_EXTENSIONS setting has '
+                       'changed. It should now be a dict mapping '
+                       'fully-qualified extension names to their '
+                       'configurations. Falling back to the default')


missing tailing '.'

ingwinlu · 2015-11-29T08:42:25Z

other than the comments in the code, this looks good to me now.

if you want to get rid of coveralls warning you might need to add a test for the warning issued when md_extensions is still a list. see other tests that look for logged messages.

As proposed by in getpelican#1638

kernc · 2015-11-29T15:35:58Z

You're a nitpicker, aren't you. 😛
Got rid of it, thanks.

ingwinlu · 2015-11-30T10:13:29Z

I would love not have to have to correct you.

Squash is still missing, after that we are good to go @justinmayer

justinmayer · 2015-11-30T16:31:14Z

@kernc: As Winlu suggested, would you be so kind as to squash your commits? Many thanks for patiently working with us to get this ready for merge. (^_^)

kernc · 2015-11-30T16:57:26Z

Oh, no problem at all.

* Make MD_EXTENSIONS setting a dict and add tests for this change; * Short extension names ('extra', 'meta') are deprecated https://pythonhosted.org/Markdown/release-2.6.html#shortened-extension-names-deprecated * Extension config as part of extension name is deprecated https://pythonhosted.org/Markdown/release-2.6.html#extension-configuration-as-part-of-extension-name-deprecated

justinmayer · 2015-11-30T17:32:04Z

Sincere thanks to @kernc, @ingwinlu, and @avaris for your work on this. Bravo! 🎉

Avoid Markdown 2.6 deprecations

kernc reviewed Feb 23, 2015
View reviewed changes

kernc force-pushed the markdown2.6-deprecations branch from 944bea2 to f048000 Compare February 28, 2015 03:34

kernc reviewed Feb 28, 2015
View reviewed changes

kernc force-pushed the markdown2.6-deprecations branch 2 times, most recently from ae9880f to 499014a Compare February 28, 2015 03:50

justinmayer added this to the 3.7 milestone Aug 19, 2015

kernc force-pushed the markdown2.6-deprecations branch 3 times, most recently from bfd31b8 to c08866e Compare August 19, 2015 22:09

kernc force-pushed the markdown2.6-deprecations branch from c08866e to 33580b4 Compare August 20, 2015 10:03

kernc force-pushed the markdown2.6-deprecations branch 2 times, most recently from 95d1617 to 2b18a3f Compare November 8, 2015 22:29

kernc added a commit to kernc/pelican that referenced this pull request Nov 15, 2015

Make MD_EXTENSIONS setting a dict

711be33

As proposed by in getpelican#1638 (comment)

ingwinlu reviewed Nov 15, 2015
View reviewed changes

kernc force-pushed the markdown2.6-deprecations branch from 711be33 to e9259d3 Compare November 15, 2015 13:35

kernc added a commit to kernc/pelican that referenced this pull request Nov 15, 2015

Make MD_EXTENSIONS setting a dict

e9259d3

As proposed by in getpelican#1638 (comment)

kernc force-pushed the markdown2.6-deprecations branch from e9259d3 to 7e58d58 Compare November 15, 2015 16:21

kernc added a commit to kernc/pelican that referenced this pull request Nov 15, 2015

Make MD_EXTENSIONS setting a dict

7e58d58

As proposed by in getpelican#1638

kernc force-pushed the markdown2.6-deprecations branch from 7e58d58 to 3ef99c9 Compare November 15, 2015 16:25

kernc added a commit to kernc/pelican that referenced this pull request Nov 15, 2015

Make MD_EXTENSIONS setting a dict

3ef99c9

As proposed by in getpelican#1638

kernc force-pushed the markdown2.6-deprecations branch from 3ef99c9 to d7c3a39 Compare November 15, 2015 17:52

kernc added a commit to kernc/pelican that referenced this pull request Nov 15, 2015

Make MD_EXTENSIONS setting a dict

d7c3a39

As proposed by in getpelican#1638

ingwinlu mentioned this pull request Nov 29, 2015

Add MD_EXTENSION_CONFIGS setting #1874

Closed

ingwinlu reviewed Nov 29, 2015
View reviewed changes

atodorov pushed a commit to atodorov/pelican that referenced this pull request Nov 29, 2015

Make MD_EXTENSIONS setting a dict

7a3607c

As proposed by in getpelican#1638

kernc force-pushed the markdown2.6-deprecations branch from d7c3a39 to 73d2a75 Compare November 29, 2015 15:10

kernc added a commit to kernc/pelican that referenced this pull request Nov 29, 2015

Make MD_EXTENSIONS setting a dict

73d2a75

As proposed by in getpelican#1638

kernc force-pushed the markdown2.6-deprecations branch from 12bd1a6 to 090ffdd Compare November 30, 2015 16:56

kernc force-pushed the markdown2.6-deprecations branch from 090ffdd to 510961b Compare November 30, 2015 17:13

justinmayer added a commit that referenced this pull request Nov 30, 2015

Merge pull request #1638 from kernc/markdown2.6-deprecations

959f96f

Avoid Markdown 2.6 deprecations

justinmayer merged commit 959f96f into getpelican:master Nov 30, 2015

Totktonada mentioned this pull request Nov 3, 2016

Unstable markdown extensions order #2044

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid Markdown 2.6 deprecations #1638

Avoid Markdown 2.6 deprecations #1638

kernc commented Feb 23, 2015

kernc Feb 23, 2015

ingwinlu commented Feb 24, 2015

ingwinlu commented Feb 24, 2015

kernc commented Feb 25, 2015

avaris commented Feb 25, 2015

kernc commented Feb 27, 2015

kernc Feb 28, 2015

kernc commented Feb 28, 2015

justinmayer commented May 2, 2015

avaris commented May 2, 2015

ingwinlu commented May 3, 2015

ingwinlu commented Aug 18, 2015

justinmayer commented Aug 19, 2015

kernc commented Aug 19, 2015

ingwinlu commented Aug 19, 2015

kernc commented Aug 24, 2015

justinmayer commented Sep 2, 2015

justinmayer commented Nov 5, 2015

kernc commented Nov 8, 2015

ingwinlu Nov 15, 2015

kernc Nov 15, 2015

avaris commented Nov 15, 2015

kernc commented Nov 15, 2015

ingwinlu Nov 29, 2015

ingwinlu commented Nov 29, 2015

kernc commented Nov 29, 2015

ingwinlu commented Nov 30, 2015

justinmayer commented Nov 30, 2015

kernc commented Nov 30, 2015

justinmayer commented Nov 30, 2015

Avoid Markdown 2.6 deprecations #1638

Avoid Markdown 2.6 deprecations #1638

Conversation

kernc commented Feb 23, 2015

kernc Feb 23, 2015

Choose a reason for hiding this comment

ingwinlu commented Feb 24, 2015

ingwinlu commented Feb 24, 2015

kernc commented Feb 25, 2015

avaris commented Feb 25, 2015

kernc commented Feb 27, 2015

kernc Feb 28, 2015

Choose a reason for hiding this comment

kernc commented Feb 28, 2015

justinmayer commented May 2, 2015

avaris commented May 2, 2015

ingwinlu commented May 3, 2015

ingwinlu commented Aug 18, 2015

justinmayer commented Aug 19, 2015

kernc commented Aug 19, 2015

ingwinlu commented Aug 19, 2015

kernc commented Aug 24, 2015

justinmayer commented Sep 2, 2015

justinmayer commented Nov 5, 2015

kernc commented Nov 8, 2015

ingwinlu Nov 15, 2015

Choose a reason for hiding this comment

kernc Nov 15, 2015

Choose a reason for hiding this comment

avaris commented Nov 15, 2015

kernc commented Nov 15, 2015

ingwinlu Nov 29, 2015

Choose a reason for hiding this comment

ingwinlu commented Nov 29, 2015

kernc commented Nov 29, 2015

ingwinlu commented Nov 30, 2015

justinmayer commented Nov 30, 2015

kernc commented Nov 30, 2015

justinmayer commented Nov 30, 2015