Repository: Python-Markdown/markdown
Pull request #970 (2020-05-20 02:36:17)

Support unicode ids in toc
I'd like to propose a change to the way `slugify` works in `toc.py`.

Currently, `slugify` uses ascii to encode/decode strings and as a result if a header only contains unicode characters the resulting `id` and `href` becomes the default `#_1`, `#_2`, etc.

Instead, as the HTML standard does not impose any particular restriction on the character set used for the `id` attribute (https://html.spec.whatwg.org/multipage/dom.html#the-id-attribute), I think it would be sensible to instead use utf-8 for encoding/decoding strings in `slugify` in order to better support international input.

waylan: To make this change would mean that many users existing links would break the next time they update to the latest version of Markdown. I'm not comfortable with that.

However, the added functionality has value. I see a few ways forward:

1. The existing option available to everyone is to provide the extension with your own `slugify` function. We don't need to change anything.
2. Provide this new functionality as an option which users can enable through the extensions configuration.
facelessuser: Providing a Unicode slugify wouldn't be a bad idea (we live in a Unicode world). I know pymdown-extensions provides a number of variants so users can just pick the one they like: https://facelessuser.github.io/pymdown-extensions/extras/slugs/#alternate-slugify. It probably wouldn't be a bad idea for Python Markdown to have an optional Unicode variant, and if people want something above and beyond that, they need to provide their own function or use a publicly available one.
Hsn723: @waylan @facelessuser Thank you for the quick feedback.
Indeed this change would introduce a noticeable breaking change. If possible, I'd like to implement option 2 since option 1 may not always be approachable for end users.

For instance, this functionality could be provided through a `encoding: utf-8` configuration flag and having `slugify` accept it as an optional argument:
```
def slugify(value, separator, encoding='ascii')
```

If that sounds acceptable, I'll go ahead and implement the changes.
waylan: > For instance, this functionality could be provided through a `encoding: utf-8` configuration flag and having `slugify` accept it as an optional argument:

That is certainly one possible solution. Another might be to provide a separate slugify function which the user could pass into the `slugify` config option. Note that we document that any third party functions must accept two arguments `value` and `separator`. If we start passing a third option, that would break any existing third party functions. In other words, by simply providing a separate function, we don't change/break the existing API. Also, that keeps the number of ways to alter slugify at one.

In practice, defining two functions may not seem very DRY. Perhaps we could create a single function which accepts the third argument `encoding` and then have two wrappers which only accept  `value` and `separator`. On would call the base function with `ascii` and the other with `utf-8`.
Hsn723: @waylan 
Got it. I went ahead and implemented the suggested changes. Please take a look.
waylan: Looks good. Now we need documentation.  

<sub>Sent with <a href="http://githawk.com">GitHawk</a></sub>
Hsn723: @waylan Sorry for the delay. I've updated the documentation.
USSX-Hares: I would like to ask to add the `slugify_encoding` configuration parameter as the `slugify` parameter of function type cannot be loaded properly from the text configuration.

For backwards compatibility, when the configured encoding matches the default encoding, the extra argument may not be passed to the `slugify()` method.

Example:
```python
DEFAULT_ENCODING = 'ascii'


def _slugify(value, separator, encoding=DEFAULT_ENCODING):
    value = unicodedata.normalize('NFKD', value).encode(encoding, 'ignore')
    value = re.sub(r'[^\w\s-]', '', value.decode(encoding)).strip().lower()
    return re.sub(r'[%s\s]+' % separator, separator, value)


def slugify(value, separator):
    """ Slugify a string, to make it URL friendly. """
    return _slugify(value, separator)


def slugify_unicode(value, separator):
    """ Slugify a string, to make it URL friendly. """
    return _slugify(value, separator, 'utf-8')

<...>
class TocTreeprocessor(Treeprocessor):
    def __init__(self, md, config):
        super().__init__(md)
<...>
        self.slugify = config["slugify"]
        self.slugify_encoding = config["slugify_encoding"]
        self.sep = config["separator"]
<...>

    def run(self, doc):
<...>
                # Do not override pre-existing ids
                if "id" not in el.attrib:
                    innertext = unescape(stashedHTML2text(text, self.md))
                    slugify_kwargs = {}
                    if self.slugify_encoding != DEFAULT_ENCODING:
                        slugify_kwargs['encoding'] = self.slugify_encoding
                    el.attrib["id"] = unique(self.slugify(innertext, self.sep, **slugify_kwargs), used_ids)

<...>
class TocExtension(Extension):

    TreeProcessorClass = TocTreeprocessor

    def __init__(self, **kwargs):
        self.config = {
<...>
            "slugify": [_slugify,
                        "Function to generate anchors based on header text - "
                        "Defaults to the headerid ext's slugify function."],
            "slugify_encoding": [DEFAULT_ENCODING,
                                 "If set to non-default, will use the custom "
                                 "slugify encoding"],
            'separator': ['-', 'Word separator. Defaults to "-".'],
<...>
```

waylan: > I would like to ask to add the `slugify_encoding` configuration parameter as the `slugify` parameter of function type cannot be loaded properly from the text configuration.

Then don't use text configuration. In fact, our documentation shows preference for creating an instance of an extension in Python code. The text-based methods are only provided for backward compatibility. Even the [CLI][1] includes support for a `-c/--extension_configs` option which accepts a YAML config file. In your YAML file you can [point at a Python function][2] (`!!python/name:markdown.extensions.toc.slugify_unicode`).

I realize that this is more complicated for the user than setting a `slugify_encoding` setting. But I'm only interested in implementing one way to set the encoding. If you can provide an alternate solution that does that, I'll consider it. However, as we already support the option for the user to provide their own slugify function, I expect that any such solution would always be a second way to address the issue.

Finally, your suggested solution requires that a third parameter be passed to the slugify function. However, that would break any existing third party functions which only accept two parameters. To maintain backward compatibility, we can only pass two parameters to slugify, Therefore, there is no way to pass a `slugify_encoding` configuration option to the function without breaking existing users' custom functions.

[1]: https://python-markdown.github.io/cli/#using-extensions
[2]: https://pyyaml.org/wiki/PyYAMLDocumentation#names-and-modules
USSX-Hares: @waylan did not know there is an option to point the function reference through YAML.
Thank you.
waylan: If an entry is added to the [release notes][1], I'll merge this.

[1]: https://github.com/Python-Markdown/markdown/blob/b701c34ebd7b2d0eb319517b9a275ddf0c89608d/docs/change_log/release-3.3.md#new-features

Patch:
diff --git a/docs/change_log/release-3.3.md b/docs/change_log/release-3.3.md
index ab7a7c6..cf4cce5 100644
--- a/docs/change_log/release-3.3.md
+++ b/docs/change_log/release-3.3.md
@@ -81,6 +81,13 @@ The following new features have been included in the 3.3 release:
   maintain the current behavior in the rebuilt Markdown in HTML extension. A few random
   edge-case bugs (see the included tests) were resolved in the process (#803).
 
+* An alternate function `markdown.extensions.headerid.slugify_unicode` has been included
+  with the [Table of Contents](../extensions/toc.md) extension which supports Unicode
+  characters in table of contents slugs. The old `markdown.extensions.headerid.slugify`
+  method which removes non-ASCII characters remains the default. Import and pass
+  `markdown.extensions.headerid.slugify_unicode` to the `slugify` configuration option
+  to use the new behavior.
+
 ## Bug fixes
 
 The following bug fixes are included in the 3.3 release:
diff --git a/docs/extensions/toc.md b/docs/extensions/toc.md
index b068b29..8dce335 100644
--- a/docs/extensions/toc.md
+++ b/docs/extensions/toc.md
@@ -202,6 +202,9 @@ The following options are provided to configure the output:
 
     The callable must return a string appropriate for use in HTML `id` attributes.
 
+    An alternate version of the default callable supporting Unicode strings is also
+    provided as `markdown.extensions.headerid.slugify_unicode`.
+
 * **`separator`**:
     Word separator. Character which replaces white space in id. Defaults to "`-`".
 
diff --git a/markdown/extensions/toc.py b/markdown/extensions/toc.py
index b6cdc73..b2564c9 100644
--- a/markdown/extensions/toc.py
+++ b/markdown/extensions/toc.py
@@ -23,11 +23,16 @@ import unicodedata
 import xml.etree.ElementTree as etree
 
 
-def slugify(value, separator):
+def slugify(value, separator, encoding='ascii'):
     """ Slugify a string, to make it URL friendly. """
-    value = unicodedata.normalize('NFKD', value).encode('ascii', 'ignore')
-    value = re.sub(r'[^\w\s-]', '', value.decode('ascii')).strip().lower()
-    return re.sub(r'[%s\s]+' % separator, separator, value)
+    value = unicodedata.normalize('NFKD', value).encode(encoding, 'ignore')
+    value = re.sub(r'[^\w\s-]', '', value.decode(encoding)).strip().lower()
+    return re.sub(r'[{}\s]+'.format(separator), separator, value)
+
+
+def slugify_unicode(value, separator):
+    """ Slugify a string, to make it URL friendly while preserving Unicode characters. """
+    return slugify(value, separator, 'utf-8')
 
 
 IDCOUNT_RE = re.compile(r'^(.*)_([0-9]+)$')


Test patch:
diff --git a/tests/test_syntax/extensions/test_toc.py b/tests/test_syntax/extensions/test_toc.py
index 3fc9780..a3d050c 100644
--- a/tests/test_syntax/extensions/test_toc.py
+++ b/tests/test_syntax/extensions/test_toc.py
@@ -141,3 +141,25 @@ class TestTOC(TestCase):
             '</h1>',                                                              # noqa
             extensions=[TocExtension(permalink=True, permalink_title="")]
         )
+
+    def testPermalinkWithUnicodeInID(self):
+        from markdown.extensions.toc import slugify_unicode
+        self.assertMarkdownRenders(
+            '# Unicode ヘッダー',
+            '<h1 id="unicode-ヘッター">'                                                            # noqa
+                'Unicode ヘッダー'                                                                  # noqa
+                '<a class="headerlink" href="#unicode-ヘッター" title="Permanent link">&para;</a>'  # noqa
+            '</h1>',                                                                                # noqa
+            extensions=[TocExtension(permalink=True, slugify=slugify_unicode)]
+        )
+
+    def testPermalinkWithUnicodeTitle(self):
+        from markdown.extensions.toc import slugify_unicode
+        self.assertMarkdownRenders(
+            '# Unicode ヘッダー',
+            '<h1 id="unicode-ヘッター">'                                                            # noqa
+                'Unicode ヘッダー'                                                                  # noqa
+                '<a class="headerlink" href="#unicode-ヘッター" title="パーマリンク">&para;</a>'    # noqa
+            '</h1>',                                                                                # noqa
+            extensions=[TocExtension(permalink=True, permalink_title="パーマリンク", slugify=slugify_unicode)]
+        )