parsedown
parsedown copied to clipboard
erusev
Create ID's for Header elements so they can be referenced in anchor tags
This allows rendered markdown to be compatible when viewed via parsed markdown or GitHub
Closes #710
I was reading how league/commonmark does the slugs, and it turns out to be stripping a lot characters as well, so I sent a PR https://github.com/thephpleague/commonmark/pull/467
I was reading how
php-league/commonmarkdoes the slugs, and it turns out to be stripping a lot characters as well, so I sent a PR thephpleague/commonmark#467
Didn't even know about that one 👍
so the tests are failing now because the leading # is no longer applicable. The question is, should we trim the leading hyphen?
Hi @netniV and @Ayesh thanks for the great work you've done towards these changes to arrive at a very sensible slug function :)
The topic of slugs (or header ID's) have come up a few times in the past (e.g.), and the position has previously been that there wasn't a universal choice that everyone using the library would want, so to avoid making the choice for everyone we'd leave it alone and up to extensions.
I've built upon the changes you've worked on here in PR #767, which modifies these changes for the new 2.0.x branch, as well as adding the option for a user-defined slug function (so that anyone who doesn't like the default can specify their own).
@netniV I've listed you as a co-author in the commit to that PR (which I hope is okay?) because most of the work has been done here in arriving at a sensible slug function to use.
Looks good. I added one comment as per our discussions here over str_replace. More involved is your change but I was thinking it should be an optional addition as it could affect other elements due to a duplication of ID
The only other thought is with trimming a trailing and leading hyphen as that seems wrong to keep those to me.
Does this mean this PR should now be closed as it will only be applied to the 2.0 version?
@aidantwoods - is there a timeframe for when 2.x will be released? I'd rather have my users use the auto IDs for headers than enable ParsedownExtra.
It would be nice if this PR could be merged into the 1.8.x branch and a 1.8.1 released. Either that, or have 2.0 released. At the moment, I can't get these changes through composer without publishing my own version of the package.
I don't think drastic changes like this would be realistically merged to the 1.x branch. What I do is simply extend the Parsedown class, and override the necessary parts. It turned out a good approach long term because I wanted to add some extra markup on my own too.
Let me see if I can manage that. I did something for images so it may be possible.
Personally, I wouldn't call it a drastic change, but I can see ID conflicts could break things so I see where you are coming from. I would submit a forked version with it in, but that seems a waste for such a small feature.
Firstly, I think it would be good to have this 1.8.0-beta7 as a full release given there has been no other changes in the past year that I can see (Aug 2020). Requiring a beta version in a composer.json looks ... well bad to me. The reason that I need to use the beta version is for handler array usage.
Now that I have fixed on the latest versions, I am still getting a problem which is due to changes in this beta version causing an issue with your parsedown-extra class.
https://github.com/netniV/ParsedownID/issues/1
@Ayesh or @aidantwoods do you have the ability to sort out the current mess of Parsedown vs Parsedown-Extra ? Due to the beta changes that haven't been published as a version, parsedown-extra relies on older versions. But to use the newer array-based handler definitions, you need this beta version and parsedown-extra apparently hasn't been updated in two years so can't handle it.
It's been a while, but now very close to a finished product for 2.0 in Parsedown (https://github.com/erusev/parsedown/pull/708) and ParsedownExtra (https://github.com/erusev/parsedown-extra/pull/172). I'd appreciate any feedback you have :)
I will try and see if I have time. Things are rather busy at the moment though.