Footnotes separator logic needs revision for 3.0

[avaris]

Since output_format is now limited to html or xhtml, this if branch in footnotes extension will never be True. Therefore a different output is generated compared to the previous versions if you had html5 or xhtml5, and that is causing test errors (see getpelican/pelican#2414).

It is not clear to me which of the separator options is preferable now, so I'm hesitant to adjust the test outputs in pelican prematurely.

[waylan]

The alternate separator behavior seems to have been introduced as a solution for #180 in 41cc055. I had completely forgot about this when reducing the output formats (in #633). For the record, after 41cc055 any formats prior to HTML5 continued to use the old format (a colon as a separator in the footnote ids), while HTML5 switched to using a hyphen as the separator.

Of course, with the 3.0 release, we reduced the output formats to html and xhtml with the understanding that both formats output HTML5 (in the HTML or XHTML flavors respectively). Therefore, the footnote ids should presumably be using the hyphen as a separator.

The problem is that that is not backward compatible for users who either intentionally chose pre-HTML5 formats or who previously were not picky and used the non-version specific format (which used to default to a pre-HTML5 format). In other words, whichever way we go, we will break things for half of our users. So the question is: which half do we break things for?

[waylan]

Thinking about this further I am reminded that the footnotes extension had always been intended as a clone of PHP Markdown Extra's footnote behavior. In fact, the one notable difference was that we used a hyphen rather than a colon as a separator in the footnote ids for HTML5 output. As PHP Markdown does not have an HTML5 specific output format, that has never been an option for them. Additionally, there is nothing in the HTML5 spec which prevents a colon from being part of an id. Therefore, I am inclined to leave the behavior as-is, that is, only use a colon as the separator.

However, it is reasonable to expect that some users will want the hyphen rather than the colon in their output. It is even possible that a few users have specifically chosen one of the HTML5 specific output formats for this reason alone. Therefore, it makes sense to add a new configuration option which allows the user to define their own separator (the default being a colon).

[avaris]

👍
I think a configuration option is a good solution.

[facelessuser]

I was looking to take care of this and I saw this in the footnote code:

            if self.footnotes.md.output_format not in ['html5', 'xhtml5']:
                a.set('rel', 'footnote')  # invalid in HTML5

So, should we just rip this out? It used to not be set if html5 was set, but now there is just the html setting, but most HTML these days are HTML5.

[facelessuser]

It seems that rel is still okay in HTML5. rev is marked obsolete. Do we want to leave rel in and replace rev instances with rel?

[waylan]

It seems that rel is still okay in HTML5. rev is marked obsolete. Do we want to leave rel in and replace rev instances with rel?

This is directly related to michelf/php-markdown#58. As our implementation is a clone of PHP Markdown Extra, let's do whatever that implementation does. Personally I had forgotten that we had different behavior for HTML5 versus earlier formats.

[facelessuser]

Looks like they are just using classes footnote-ref and footnote-backref. Pretty straight forward to change.

I'll take this on and just update footnotes to generally stop looking to the output format for specific changes. A configuration will added called SEPARATOR (because everything in the footnotes extension uses uppercase for some reason) which will control the separator.

[waylan]

because everything in the footnotes extension uses uppercase for some reason

Footnotes was the first extension ever and existed before I got involved in the project. There are some unusual (or at least unpythonic) naming conventions in the old code (such as mixedCase and/or CamelCase rather than lowercase_with_underscores for method names etc). We really should have changed all that in the 3.0 release, but in the end, backward compatibility won out over style.

What i'm not sure about is what style we should use for newer additions. Should we just copy the existing style for consistency or follow a modern style guide? The footnotes extension is the worst offender here. Part of me wants to completely rework it but that might be a little to user hostile. I suppose, in the end I'll accept whatever color you paint this shed.

[facelessuser]

I kind of wish we could normalize input configurations key names and just use lower case internally. As far as footnotes go, I don't really have a preference. I'd be happy to use lowercase for new options regardless of what previous options were, but I figured I'd go with the status quo in case people wanted to complain about it.

[facelessuser]

Unfortunately normalization would never work as 3rd party extensions can't be controlled.

[waylan]

I was going to suggest that we could normalize the options in the config handling methods of the Extension class, but then I realized that the extensions also look up those config key names and that would break...

Unless we override the config lookup as well. Provide our own __getitem__ method which normalized key names to lowercase before lookup. Of course, that sort of change would be separate from this PR.

[facelessuser]

That's an interesting idea. If we were overriding the getter, we could also generate deprecation warnings...

Something to play around with as a separate issue.