TOC extension: Add new boolean option permalink_prepend

[ferdnyc]

This PR adds a new boolean to the TOC extension configuration. As described in the documentation:

• permalink_prepend:
  Set to True if the generated permalinks should be placed at the
  start of the header, rather than at the end. Default is False.

Basically, if the output of the conversion with permalink set to True would be this:

<h1 id="header-1">
Header 1
<a class="headerlink" href="#header-1" title="Permanent link">&para;</a>
</h1>
<h2 id="header-2">
Header 2
<a class="headerlink" href="#header-2" title="Permanent link">&para;</a>
</h2>

Then, also setting permalink_prepend to True would transform that into this:

<h1 id="header-1">
<a class="headerlink" href="#header-1" title="Permanent link">&para;</a>
Header 1
</h1>
<h2 id="header-2">
<a class="headerlink" href="#header-2" title="Permanent link">&para;</a>
Header 2
</h2>

This creates permalinks similar to the GitHub-Flavored Markdown converter's output, allowing them to be styled in the manner of list-marker symbols: They can be placed slightly outside of the heading's own content by means of a hanging indent created with negative margins.

(See, for example, the section link anchors in this repo's own README.md, as rendered at https://github.com/Python-Markdown/markdown/.)

Documentation updates are included for the new option, along with four new tests added to the TOC extension suite:

• A test of permalink by itself, on a simple text header
• A test of permalink with permalink_prepend, on a simple text header
• A test of permalink alone, on a header with inline markup (`fenced code`)
• A test of permalink + permalink_prepend, on a header with inline markup (`fenced code`)

All four tests are passing and the extension's coverage remains at 100%.

The contributing guide also mentions adding a note to the release notes, but I checked out docs/change_log/index.md and got the impression that it was probably autogenerated, so I didn't touch it. If that's wrong, I'll happily add a line about this change.

[waylan]

Interesting feature request. Can't say that it would have ever occurred to me to add this. Although, I do have one question: is this necessary? Isn't there a way to accomplish this with CSS alone? I realize the CSS would be much more complex that a simple setting but I'm just making sure it makes sense to add this.

I also hesitate regarding the name of the feature: permalink_prepend. It was not clear to me what the option did from the name alone. Therefore, I wonder if perhaps we could find a better name for it. But then again, as I mentioned, this feature had not occured to me, so maybe that's the problem and the name is fine.

The contributing guide also mentions adding a note to the release notes

Yeah, our guide probably needs more details, but thank you for checking. The change_log is manually edited. As you are adding a new feature, this would then need to be part of a point release (3.5). As no release notes yet exist for that version, you would need to create the file at docs/change_log/release-3.5.md and then add a reference to it both in docs/change_log/index.md and the nav section of mkdocs.yml.

[waylan]

The code looks good. If you add release notes and address the spelling issue, this could be ready to go assuming my other concerns are satisfied.

[ferdnyc]

@waylan

Interesting feature request. Can't say that it would have ever occurred to me to add this. Although, I do have one question: is this necessary? Isn't there a way to accomplish this with CSS alone? I realize the CSS would be much more complex that a simple setting but I'm just making sure it makes sense to add this.

Fair question. I don't believe there's any clean way to accomplish this with CSS alone, no matter how complex. The side-effects of having the link tag at the end of the header content are too great to easily overcome. (For example, the trailing tag could be positioned to the left of the heading... but if it had been word-wrapped to a second line, it would remain aligned with the second line of the heading, instead of the first as intended.)

As noted, I took inspiration from GitHub's own Markdown rendering, which places the permalink <a> tag at the front of the heading. My impression is, that's done not merely because it's convenient, but because it's necessary. I'm happy to be proven wrong, but if there's a way to do this with CSS I don't know it.

I also hesitate regarding the name of the feature: permalink_prepend. It was not clear to me what the option did from the name alone. Therefore, I wonder if perhaps we could find a better name for it. But then again, as I mentioned, this feature had not occured to me, so maybe that's the problem and the name is fine.

I thought about that, too. I should've included some proposals for alternatives in the original PR note. Such as:

• permalink_leading
• permalink_start
• permalink_front (← I actually hate that one, but for completeness' sake...)

The contributing guide also mentions adding a note to the release notes

Yeah, our guide probably needs more details, but thank you for checking. The change_log is manually edited. As you are adding a new feature, this would then need to be part of a point release (3.5). As no release notes yet exist for that version, you would need to create the file at docs/change_log/release-3.5.md and then add a reference to it both in docs/change_log/index.md and the nav section of mkdocs.yml.

Gotcha, thanks. I'll take care of that and any other changes requested.

[waylan]

• permalink_leading
• permalink_start

I would be okay with either of those. I think I prefer them to prepend. I might add leading_permalink, permalink_at_start, and permalink_first to the list. I don't have any specific preference.

I think maybe the issue I have with prepend is that the setting does not add/insert a permalink (which the word prepend suggests), but rather moves the permalink from its default location.

[ferdnyc]

Fair question. I don't believe there's any clean way to accomplish this with CSS alone, no matter how complex. The side-effects of having the link tag at the end of the header content are too great to easily overcome. (For example, the trailing tag could be positioned to the left of the heading... but if it had been word-wrapped to a second line, it would remain aligned with the second line of the heading, instead of the first as intended.)

As a little more on this, the PR grew out of an issue with gi-docgen. (That's the API documentation generator for GObject Introspection, responsible for sites like https://docs.gtk.org/.) We'd been using display:flex on our heading tags, just to reliably align the permalink to the right of the heading even in multi-line situations.

That works fine, for simple headings. But, here's what was happening when that display:flex bumped up against a more complex heading that contains inline <code> tags:

(I've since discovered that styling the <code> tags inside the headers with display:contents can overcome that issue, by doing what display:inline was supposed to mean, but doesn't inside a flexbox container. But there are apparently some accessibility issues with display:contents, and I'm not sure it's a great solution for this, even though it works to visually fix the problem.)

So, that aside, we've chosen to just remove the display:flex on the headings for now, and are being content with this wrapping:

But we'd really prefer to move the permalinks slightly outside the heading, list-marker style, and having them at the start makes that trivial and is supported everywhere; give them a negative left margin and you're done, basically.

[ferdnyc]

@waylan

I might add leading_permalink,

I quickly decided to avoid anything that didn't start with permalink_, simply because there are already two other permalink_foo options. It seemed like breaking that pattern would be unnecessarily confusing, and worse, could feel intentionally confusing.

I've changed it to permalink_leading, and tweaked the description to:

• permalink_leading:
  Set to True if the extension should generate leading permanent links.
  Default is False.

  Leading permanent links are placed at the start of the header tag,
  before any header content. The default permalink behavior (when
  permalink_leading is unset or set to False) creates trailing
  permanent links, which are placed at the end of the header content.

The only remaining uncertainty I have is whether I should make { "permalink_leading": 1 } a synonym for { "permalink": 1, "permalink_leading": 1 } (IOW, automatically enable permalink if permalink_leading is explicitly enabled), or if I should just ignore permalink_leading without permalink.

[waylan]

The only remaining uncertainty I have is whether I should make { "permalink_leading": 1 } a synonym for { "permalink": 1, "permalink_leading": 1 } (IOW, automatically enable permalink if permalink_leading is explicitly enabled), or if I should just ignore permalink_leading without permalink.

Definitely ignore permalink_leading without permalink. That is how the other permalink options work so we should be consistent.

Also, thank you for the detailed explanation of the CSS issue. I'm not surprised by it, but wanted to at least ensure some thought was given to it before accepting this. This is clearly the better solution.

[ferdnyc]

@waylan OK, I believe I've addressed everything and this should be ready for re-review. Thanks for all of your guidance on this!

[waylan]

It looks like the spellchecker is failing on your code comments. We use Markdown syntax in our comments. You may need to add a few code spans to your comments.

Also, please don't force push. Instead push new commits which address the requested changes. Then we can review those changes in isolation if need be. It hasn't been an issue in this PR, but that is best practice. We will squash the commits when we merge.

[waylan]

Thanks for your work on this. It is ready to go. However, I am not sure when we will be making our next point release. Therefore I am not going to merge this immediately. I have added it to the 3.5 Milestone so we won't forget about it.