While working on this, I noticed that Python-Markdown doesn't correctly un-stash HTML for .toc_tokens. I filed that as Python-Markdown/markdown#899. Once that gets fixed, I'll finish this patch up.

While working on this, I noticed that Python-Markdown doesn't correctly un-stash HTML for .toc_tokens. I filed that as Python-Markdown/markdown#899. Once that gets fixed, I'll finish this patch up.

This means that we will need to require the most recent version of Markdown. Traditionally we have supported much older Markdown versions. I wonder if we should leave the old HTML to TOC parser as a fallback. If the new version of Markdown is installed, use the toc_tokens, but if an older version is installed, pass the HTML toc through the _parse_html_table_of_contents function and use the output of that instead. We might even issue a DeprecationWarning informing the user to upgrade their version of Markdown.

On the other hand, we could just update the required version in setup.py, which would force Markdown to be updated on install.

One or the other needs to happen. Either way, it should be part of this PR. Let's see what happens with the Markdown fix first.

This means that we will need to require the most recent version of Markdown.

A third option would be to manually strip out the stashed HTML placeholders from toc_tokens if Python-Markdown isn't up-to-date. That should be equivalent to what the code does with toc, and would be more similar to the proper way using the most recent (future) version of Markdown.

Did you close this intentionally? I thought this was a good start

_{Sent with GitHawk}

Whoops, I think I unintentionally deleted this branch when I was cleaning up my repo. Let's see if I can undo that...

FYI, Markdown 3.2 has just been released, which includes the fix this is waiting for (Python-Markdown/markdown#901).

Cool, I'll work on finishing this up, hopefully by the end of the weekend. @waylan, if you're still concerned about requiring the latest version of Python-Markdown in MkDocs, I could work around that by stripping the stashed HTML placeholders from the TOC object. It should just be a simple regex. I'd prefer (not strongly) to just require Python-Markdown 3.2 though.

Requiring Markdown 3.2 is fine. As it turns out, the upcoming release of MkDocs and Markdown 3.2 are both the first versions to drop support for Python 2.7, so it makes sense that MkDocs updates it's dependency anyway.

@waylan Ok, this is mostly good. I did notice one weird thing that's causing a test failure though: if you have a header like # foo > ", Python-Markdown 3.2 returns that as 'foo > "' in .toc_tokens (3.1.1 returns 'foo > \x02wzxhzdk:0\x03' because it's not unstashing the HTML). On the other hand, the HTML in .toc would be foo > ", which is easier for end users to work with. This makes it a bit tough to escape the text properly for use in the templates, since we'd have to use Markdown's fancy HTML escaping logic.

This seems like a bug in Python-Markdown, since it does strip Markdown inside the header for .toc_tokens (i.e. # foo *bar* becomes 'foo bar'). This case wasn't previously covered in the Python-Markdown tests, so it never even occurred to me that this is what we'd end up with until I went to finish this PR... :/

This seems like a bug in Python-Markdown

Yes, it is. I've reported it at Python-Markdown/markdown#906.

Python-Markdown 3.2.1 was just released with a fix for the escaping issue. That needs to be the minimum version supported by MkDocs now.

@jimporter this looks good. Awesome job. I just want to make sure the change to have TOC items being 1-indexed is not going to be a problem for third party themes. @squidfunk do you have any input on this? The idea is that the TOC level attribute will now always match n in Hn for all headers (H1-H6). I don't intend to change the behavior back, but should we be advertising this as a backward incompatible change?

@waylan it’s been a long time but I have a hack in place that detects whether there is an h1 in the content of the page to otherwise use the page title as h1, because as I recall level didn’t provide that information as it was zero-indexed. It effectively meant that I could not use this information, as it was always relative to the lowest hn. Is that what you mean? If so, it would not be a backward incompatible change, because it’s more of a feature, since we now have something that we didn’t have before. I could finally get rid of that hack.

@squidfunk thanks for the feedback. And we're glad to provide a useful feature.