parsedown-extra icon indicating copy to clipboard operation
parsedown-extra copied to clipboard

Unique Footnote anchors

Open graphidev opened this issue 9 years ago • 19 comments

As I am using Parsedown-extra many times in a same page, the anchors target not only the concerned item. E.g:

` Item 1

fn:1

fnref1:1

Item 2

fn:1

fnref1:1

`

As you can the see, this always refer to a same item footnote (the first one). So is it possible to generate an unique id for footnotes ?

graphidev avatar Mar 15 '15 08:03 graphidev

I believe it is possible.

In fact, it shouldn't be hard to implement.

Would anyone be willing to create a PR?

erusev avatar Mar 15 '15 22:03 erusev

Wait, I just thought that footnotes could serve as permalinks. So they shouldn't have to be unique (as PHP uniq built-in function) at each Parsedown-extra parsing.

I will think about this and make a PR

graphidev avatar Mar 16 '15 06:03 graphidev

So they shouldn't have to be unique (as PHP uniq built-in function) at each Parsedown-extra parsing.

I'm not sure I understand.

My solution would be to keep the footnote data in a class (static) variable. This would make sure that the numbers of footnotes are unique among different instances of the parser.

erusev avatar Mar 16 '15 12:03 erusev

My bad.

I was thinking of a blog architecture where posts are ordered by descending date.

The fn1:1 would target the first foot note of the first post. However, when a new post is available, then the permalink http://example.com/#fn1:1 will no more refer to the right post (should refer to the second one).

Maybe this could be available as an option to set a custom runtime id. Then the foot notes ids could be something like post456-fn1:23

graphidev avatar Mar 16 '15 12:03 graphidev

I think the fix made by @robinadr (a234bbeb4d25fe9bb6d40ceb3df0945f981636f7) is a good answer to this probleme. Could it be merged with parsedown-extra in order to get the update from Composer ?

graphidev avatar Apr 02 '15 13:04 graphidev

Thanks, I'll have a look as soon as I have some time.

erusev avatar Apr 02 '15 13:04 erusev

I'm debating whether to either add a clearFootnotePrefix() method or make it automatically clear after each text rendering. That being said, it doesn't seem any of the other setters are one time use only.

Anybody have any feedback on this?

robinadr avatar Apr 02 '15 15:04 robinadr

It would be weird to use twice a same ID (or prefix, as you want) :

  • There is no reason to parse twice the same text
  • Parsing various texts with a same ID could generate conflicts (which is the reason of this issue).

I think, in my personal opinion, that the prefix must be reseted after each text rendering. However, someone could have an other example that goes to the opposite.

graphidev avatar Apr 02 '15 15:04 graphidev

I agree. Given that this is a new feature, we have an opportunity now to change the behavior as we imagine it would work best.

Another more generic option is what was previously suggested: keep an internal count and add that on as necessary.

This is simpler but I think it fails when someone might create multiple instances of the parser for a single web page load.

I think having the prefix reset is a good idea, but I'm also wondering if that should be able to be toggled on or off. But then we run the risk of over complication.

On Apr 2, 2015, at 10:37 AM, Sébastien ALEXANDRE [email protected] wrote:

It would be weird to use twice a same ID (or prefix, as you want) :

There is no reason to parse twice the same text Parsing various texts with a same ID could generate conflicts (which is the reason of this issue). I think, in my personal opinion, that the prefix must be reseted after each text rendering. However, someone could have an other example that goes to the opposite.

— Reply to this email directly or view it on GitHub.

robinadr avatar Apr 02 '15 16:04 robinadr

You are right, both generic and custom prefix should be implemented.

Here are the behaviors I suggest :

<?php

    $parsedown = new ParsedownExtra;

    $parsedown->text('...');  # 1
    $parsedown->text('...');  # 2

    $selfReset = true;
    $parsedown->setFootnotePrefix('abcd1234', $selfReset);
    $parsedown->text('...');  # abcd1234

    # if $selfReset is FALSE, then call
    # $parsedown->resetFootnotePrefix(/* FALSE by default */) 
    # in order to reset the custom prefix

    $parsedown->text('...');  # 3 : Previous custom prefix does not reset generic one
    $parsedown->text('...');  # 4

    # Reinstanciated ParsedownExtra does not use previous generic prefix
    $anotherParsedown = new ParsedownExtra;
    $anotherParsedown->text('...'); // # 1

    # reset both generic and custom prefix
    $resetBothCustomAndGeneric = true;
    $parsedown->resetFootnotePrefix($resetBothCustomAndGeneric);
    # private $genericPrefixCount = 0
    # private $customPrefixCount = null

    $parsedown->text('...');  # 1
    $anotherParsedown->text('...');  # 2 : not affected by the first instance reset

    # ...

?>

Keeping generic prefix in memory for all the Parsedown instances may generate some conflicts, such as incrementing count for $anotherParsedown instance when it should not.

graphidev avatar Apr 03 '15 11:04 graphidev

I feel that's going a little too far. Parsedown is a backend library, so I think it's fair to assume the developer using it can be held responsible to prefix footnotes as necessary. This all depends on the processing of the page (especially multiple parsing passes to be displayed on the same page) as well as how they've worked their code (same parser instance or multiple).

With all these factors dependent on the developer, I think it's most flexible to add a prefixing capability, then let the developer deal with it how they want.

On Apr 3, 2015, at 6:13 AM, Sébastien ALEXANDRE [email protected] wrote:

You are right, both generic and custom prefix should be implemented.

Here are the behaviors I suggest :

text('...'); # 1 $parsedown->text('...'); # 2 $selfReset = true; $parsedown->setFootnotePrefix('abcd1234', $selfReset); $parsedown->text('...'); # abcd1234 # if $selfReset is FALSE, then call # $parsedown->resetFootnotePrefix(/* FALSE by default */) # in order to reset the custom prefix $parsedown->text('...'); # 3 : Previous custom prefix does not reset generic one $parsedown->text('...'); # 4 # Reinstanciated ParsedownExtra does not use previous generic prefix $anotherParsedown = new ParsedownExtra; $anotherParsedown->text('...'); // # 1 # reset both generic and custom prefix $resetBothCustomAndGeneric = true; $parsedown->resetFootnotePrefix($resetBothCustomAndGeneric); # private $genericPrefixCount = 0 # private $customPrefixCount = null $parsedown->text('...'); # 1 $anotherParsedown->text('...'); # 2 : not affected by the first instance reset # ... ``` ?>

Keeping generic prefix in memory for all the Parsedown instances may generate some conflicts, such as incrementing count for $anotherParsedown instance when it should not.

— Reply to this email directly or view it on GitHub.

robinadr avatar Apr 03 '15 15:04 robinadr

My bad, I didn't read correctly your previous comment.

So let's take care of this part only :

$selfReset = true;
$parsedown->setFootnotePrefix('abcd1234', $selfReset);
$parsedown->text('...');  # abcd1234

# if $selfReset is FALSE, then call
# $parsedown->resetFootnotePrefix(/* FALSE by default */) 
# in order to reset the custom prefix

graphidev avatar Apr 03 '15 16:04 graphidev

I’m still inclined to let the developer take care of it. We can add a resetFootnotePrefix() method if necessary (it would just be setFootnotePrefix(‘’)) but my general philosophy is to leave control up to the developer using the library. It might mean an extra line or two for them, but at the end of the day I value the flexibility. That’s just me, though.

robinadr avatar Apr 03 '15 16:04 robinadr

I pushed a change that adds clearFootnotePrefix() for now.

robinadr avatar Apr 03 '15 16:04 robinadr

I just agree with your philosophy. I apologize if my samples wasn't looking that way.

Thanks a lot for you work !

graphidev avatar Apr 06 '15 18:04 graphidev

No worries, just trying to figure out what the best approach is before it's potentially pushed into the main repo. Waiting to hear what @erusev wants to do.

robinadr avatar Apr 06 '15 18:04 robinadr

@robinadr I've been super busy lately. I'll respond as soon as have some time.

erusev avatar Apr 06 '15 19:04 erusev

Can we merge this in already? :)

wladston avatar Nov 24 '15 05:11 wladston

could someone experiment w/ the original Markdown Extra and see if this issue is addressed there

erusev avatar Nov 24 '15 10:11 erusev