extend_markdown() TypeErrors swallowed because of deprecation test.

[merriam]

Currently, an extension's extendMarkdown() raising a TypeError will be swallowed, or not reported. This occurs because the calling _extendMarkdown() assumes the TypeError occurs because an md_globals argument was also passed. This pull request modifies _extendMarkdown() to check this assumption.

Also, trying to delete an invalid key is a KeyError as opposed to a TypeError.

Have a wonderful day!

[mitya57]

Tests in tests/test_apis.py should be changed to expect KeyError instead of TypeError.

I hope none of third-party extensions relied on this wrong exception type…

[merriam]

Updated unit test

[merriam]

While nothing in the repository uses this error outside of unit tests, it cannot be proven that some project, somewhere, relies upon it. It would be a stretch to imagine why.

That said, I found a bug. Fixed it. Went back and fixed the buggy unit tests. I'm OK with it never being adopted but not OK writing more makework tests. It's a bad practice, and, if we ever meet for beers, I can give twenty years of research on it.

Other fish to fry...

[waylan]

This should get a comment in the release notes under version 3.2.2.

[merriam]

Er, Travis fails because it doesn't know KeyError and TypeError spelling?

[mitya57]

@merriam As Waylan noticed, you need to put the class names into backticks: KeyError → `KeyError`.

[waylan]

Er, Travis fails because it doesn't know KeyError and TypeError spelling?

You don't expect KeyError and TypeError to be in the English dictionary do you? Sure, they are known Python words, but then they are code and the spellchecker ignores code. So, put then in code spans where they belong.

Tests accomplish two purposes: (1) ensure we don't break our code and (2) ensure we follow our own style guide. This accomplishes the later.

[merriam]

Sorry, I'm giving up.

The code changes took about 20 minutes. Getting it accepted has taken several hours, and I have no idea how many more steps will appear. It may a joy for some, though my way is more on the development side.

The current state is that any TypeError thrown by an extendMarkdown() will cause a secondary error when it is called again with too many parameters.

[waylan]

Our process and requirements for PRs are all documented in our Contributing Guide. If we were to ignore those requirements our code and documentation would be a mess. Not sure how that would be better for anyone.

[waylan]

Oh and I would have happily made a few edits to fix the "spelling errors." However, you put the notes in the wrong file. To fix them I need to clone your fork locally, make the changes and push the changes. If you had put them in the correct location, I could have simply used GitHub's web interface to add a few backticks. I even linked to the correct location when I asked for the release notes.