RFC - page linking.

[stuaxo]

The current tag for linking pages, links by Title obviously there is the danger of clashes, is it worth making it possible to link by id or slug ?

I wonder if it's worth adding some error checking for broken links to pages in the admin interface too ?

(Obviously ideally the editor would know about the page chooser - but this might be tricky, I guess there could be something to handle pages moving / changing too, but again, a bit tricky)

[richtier]

I agree this should change because it would mean hyperlinks break if the page titles change, and that would be a blocker for many adopting this library

A solution I've implemented is to create a custom template tag that allows linking like so:

[this is a slug link](slug:cyber-security)
[this is pk link](pk:25)
[this is a normal link](http://www.google.com)

which results in

<a href="http://example.com/industries/cyber-security/">this is a slug link</a>
<a href="http://example.com/industries/cyber-security/">this is a pk link</a>
<a href="http://www.google.com">this is a normal link</a>

# templatetags.py

import markdown
from wagtail.wagtailcore.models import Page
from wagtailmarkdown.mdx import linker, tables
from wagtailmarkdown.utils import _sanitise_markdown_html

from django import template
from django.utils.safestring import mark_safe

register = template.Library()

@register.filter
def render_markdown(text, context=None):
    html = markdown.markdown(
        text,
        extensions=[
            'extra',
            'codehilite',
            tables.TableExtension(),
            LinkerExtension()
        ],
        extension_configs={
            'codehilite': [
                ('guess_lang', False),
            ]
        },
        output_format='html5'
    )
    sanitised_html = _sanitise_markdown_html(html)
    return mark_safe(sanitised_html)


class LinkPattern(markdown.inlinepatterns.LinkPattern):
    def sanitize_url(self, url):
        if url.startswith('page:') or url.startswith('pk:'):
            field, value = url.split(':')
            page = Page.objects.get(**{field: value}).specific
            url = page.url
        return super().sanitize_url(url)


class LinkerExtension(markdown.Extension):
    def extendMarkdown(self, md, md_globals):
        md.inlinePatterns['link'] = LinkPattern(
            markdown.inlinepatterns.LINK_RE, md
        )

Another advantage to doing it this way is the pattern looks very much like a traditional markdown link, so the learning curve for users is reduced, and the markdown preview knows how to render it (albeit, without the href set correctly, but aesthetically good enough):

It will also solve this problem: #41

I'm happy to make a PR for this.

[stuaxo]

These look sensible.

I don't know how many users there are of wagtail-markdown, the only thing I'd change is make slug: the default, but obviously this would break compatibility.

All in all, this looks sensible, so +1 x 100 :) [especially on the PR]

[tm-kn]

Thanks for it! I really appreciate your help.

I can agree on one - the way to reference pages by ID is needed. PR for this is definitely welcome.

The main issue with titles and slugs is that they are not unique... The links will always be ambiguous. I would suggest we have some sort of smarter resolution for titles and slugs where it's going to use pages that are the closest in the tree first.

[richtier]

Smashing :)

I'll make a PR for the primary key linking over the next few days.

[richtier]

I closed the PR due to inactivity.

[pbspm01]

Hi, this is a very nice feature, removing the pain of linking the pages. We'd love to have this officially!