gohugoio / hugo

The world’s fastest framework for building websites.
https://gohugo.io
Apache License 2.0
75.49k stars 7.51k forks source link

The Markdown Pickle #5124

Closed bep closed 5 years ago

bep commented 6 years ago

So, I've been waiting for Go 1.11/Go Modules to do the Blackfriday v2 upgrade (for versioning reasons). In the meantime, things have changed on the Go markdown scene.

The motivation behind the markdown fork was lack of maintenance/bug fixing in BF. This fork is now 490 commits ahead of its origin. Pretty fundamental changes. Mostly sensible from my brief look. But the performance has degraded a fair amount, mostly from one fundamental commit. I have talked to the lead maintainer about it, but we don't share the same worldview in this area. I can sympathize with him not being a big fan of outsiders having an opinion (I would probably have reacted the same), but it leaves us in a ... markdown pickle.

So, not using the new markdown fork would mean:

Thoughts?

/cc @kaushalmodi @regisphilibert @anthonyfok and gang

onedrawingperday commented 6 years ago

What you describe sounds like a pretty hard dilemma.

But here is something that also needs to be taken into account besides the performance issue:

A casual search for MMark in the forum returns 50+ results and right here in this repo at least 28 issues (both open and closed). It looks like that if MMark is deprecated and then removed from Hugo it would disrupt the workflow of various people.

Disclaimer: I'm not using MMark myself.

anthonyfok commented 6 years ago

Thank you for your thorough investigation!

I read through the relevant issue and could empathize with both sides of the discussion.

I am also interested in how migrating to the new markdown fork would affect Hugo's performance in the real world, but to reach that point means putting the time and effort to actual modify Hugo for the new markdown first. Maybe a bit of a gamble.

I am glad that other people like miekg and moorereason chimed-in in that issue to hopefully help move the discussion forward. And I could only wish... maybe some other kind souls could help optimize the markdown fork code and submit PRs.


Today, a new idea came into me: there might be yet another alternative, and that is to have Hugo support for all three of the following:

And, of course, let Blackfriday v2 be the default. The end user can specify the other two if they don't mind the performance degradation, as for some people, a richer Markdown syntax, e.g. that closer to CommonMark. This will bloat the Hugo executable a little bit, but not by much.

Perhaps by then people could see the performance degredation with Gomarkdown/markdown fork in real-life, and might spur the main author of the markdown into seriously optmizing the code again.

When I looked at some of the Hugo code years ago, I had a feeling that it was somewhat coupled to Blackfriday, as in when spf13 originally worked on Hugo, Blackfriday was the only supported Markdown renderer, and several functions with the name "Markdown" were actually Blackfriday specific. Though of course, the code has gone through many revisions and refactoring since then, and is already accommodating alternative renderers.

Anyhow, the integration of kjk's gomarkdown/markdown fork would be a motive for us to examine the Hugo code to find and change the code where the name "markdown" actually meant "blackfriday", and make Hugo more "renderer-agnostic".

Disclaimer: I haven't studied the relevant code in detail for a long time, and am still definitely a Go amateur, so I might be talking nonsense above.

That said, the new Blackfriday fork calling itself markdown could be utterly confusing, so when it come times to the actual integration with Hugo, whenever that maybe, may I suggest using a namespace like kjkMarkdown instead? <grin, duck, run>...

Disclaimer: Ditto. I am musing before actually looking at the code, hehe.

In summary, my suggestion is to do the following in due time:

  1. Go ahead and upgrade Hugo to Blackfriday v2.
  2. Keep Mmark v1 for the time being. IIRC, it was forked from Blackfriday v1 and the ugprade to Blackfriday v2 won't affect our current support for Mmark v1. (though I don't know if the API change in Blackfriday v2 would make our support for legacy Mmark more complicated)
  3. Add support for gomarkdown/markdown. But of course, use something other than markdown for its namespace in Hugo.
  4. Upgrade Hugo to Mmark v2.

So, to me, that would be the best of both worlds, giving the end user choices, and allowing Blackfriday v2 and the gomarkdown/markdown fork to compete with each other in real life.

Indeed, there is some cost in maintenance for yet another Markdown renderer (3 instead of 2), but hopefully the cost isn't too much.

Disclaimer: Ditto. Again, wild speculation before actually studying the relevant code...

My 2 cents. :-)

bep commented 6 years ago

I suspect you're not really aware of the implementation and maintenance cost of what you're suggesting.

My plan is to integrate Blackfriday v2 and use their AST to eventually do all sorts of cool stuff (starting with improved ToC support etc.). People would eventually expect to have a similar experience for Mmark and "Blackfriday Classic", but there is no way we're maintaining the above++ across two different BF forks.

And gomarkdown/markdown is currently not production ready. You could claim that Blackfriday is also buggy -- true, but those bugs are at least fixable.

moorereason commented 6 years ago

The big dill is that BFv2 is largely unmaintained, but I don't see any signs of that changing.

If the performance of gomarkdown was on par with BFv2, would you move to gomarkdown? If so, I'd give gomarkdown a little time (2wks?) to reconsider the performance-killing design decision that was made. If not, then let's go to BFv2 and move on.

None of us wants to drop support for mmark. That would be unfortunate if it comes to that.

bep commented 6 years ago

The big dill is that BFv2 is largely unmaintained, but I don't see any signs of that changing.

Which also has its obvious upsides.

lpar commented 6 years ago

The CommonMark Go implementation has been updated for compliance with current CommonMark standards. I'd like to at least have CommonMark as an option.

anthonyfok commented 6 years ago

@lpar, I like your suggestion too. That said, as @ bep suggested earlier in this thread, there will be a significant implementation and maintenance cost involved with supporting multiple Markdown renderers in Hugo, so please don't be disappointed if the CommonMark Go implementation at https://gitlab.com/golang-commonmark doesn't get into Hugo.

But yeah, anything could happen in the future, e.g. Blackfriday may eventually gain support for CommonMark Go too. Let's keep our fingers crossed. ;-)

jmarca commented 5 years ago

Any update or resolution of this? I hit this bug/issue in a roundabout way. I can provide my tale of woe if anyone is interested, but essentially blackfriday, pandoc, and mmark all fail me in one way or another, whereas the new mmark (or I guess, gomarkdown/markdown) work for me. I need

the new mmark processor (not the old library) does everything I need, but isn't a library...

jmarca commented 5 years ago

@bep Can you provide a hint as to the resolution as well? I'm trying to follow along to figure out the best path forward.

747 commented 5 years ago

@jmarca FYI at least this gomarkdown/markdown#95 seems where they are talking about the performance issue. Though I can judge nothing of Go benchmark details. Using Hugo has virtually nothing to do with using Go.

jmarca commented 5 years ago

Ah thanks for that pointer. Always a trade off it seems.

I can live with no block-level classes in Black Friday...there is a work-around for that case in Bulma, so the default Black Friday lib is working for me.

cyChop commented 5 years ago

Hi @bep! I've been investigating existing issues because I wished to use features from more recent versions of MMark or another (e.g. MMark's bibliography). The change of philosophy in Markdown engines' v2's is a bummer, and I can appreciate the difficulty of updating the calls to engine at the heart of a piece of software without breaking the experience for existing users.

One year later, what is the current position on this topic (changing the support from v1 to v2, or to another engine)? Is an upgrade considered or is it still impossible because to costly?

For my own curiosity (I'm not a Go developer and I've not dived deeply enough in Hugo's source code), is there somewhere an analysis (even a short one) of the impacts of moving to the v2's of blackfriday and mmark/gomarkdown (or another Markdown engine)?

I can also appreciate the importance of performance for Hugo. This is actually why I stuck to Markdown though I usually prefer to go with Asciidoctor because of its much wider set of features. After making the tests with twenty pages or so, the difference in generation time was already significant enough for me to stick with your default choices.

To close, I'd like to share some thoughts inspired from my own experiences:

dkg commented 4 years ago

fwiw, hugo's retention of mmark v1 is actually causing problems for people who want to use mmark v2 in debian. This is an unfortunate situation :(

patti24k commented 4 years ago

Please disconnect me fr git

On Sat, Nov 9, 2019, 11:53 AM dkg notifications@github.com wrote:

fwiw, hugo's retention of mmark v1 is actually causing problems for people who want to use mmark v2 in debian https://bugs.debian.org/933837. This is an unfortunate situation :(

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/gohugoio/hugo/issues/5124?email_source=notifications&email_token=ANEADGKAUD6YEAQUPO6APF3QS4BJLA5CNFSM4FR4KZCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUMTYA#issuecomment-552126944, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANEADGINAVREMCYTZSMCFG3QS4BJLANCNFSM4FR4KZCA .

bep commented 4 years ago

@dkg there is no mmark v2. The mmark that Hugo uses is stuck in v1. There is a completely new and totally different package with a different name in which Debian should feel free to use if they want.

anthonyfok commented 4 years ago

Hi @dkg, maintainer of Debian’s golang-github-miekg-mmark (i.e. mmark v1) here.

fwiw, hugo's retention of mmark v1 is actually causing problems for people who want to use mmark v2 in debian. This is an unfortunate situation :(

First of all, thank you for bringing that to my attention, as for some reason I missed Debian Bug #933837 entirely. But, just as Dawid Dziurla noted, mmark v2 at github.com/mmarkdown/mmark has an entirely different path from mmark v1 at github.com/miekg/mmark, so the packaging of golang-github-mmarkdown-mmark (v2) would have absolutely no conflict at all with the old golang-github-miekg-mmark (v1).

Since there appears to be some interests in having the new mmark packaged, I think I'll do the following (when I find some free time):

  1. [x] Merge https://bugs.debian.org/916202 and https://bugs.debian.org/933837
  2. [x] Move them to Debian wnpp (Work-Needing and Prospective Packages), changing the title to "RFP (Request for Package)" or "ITP (Intend to Package): golang-github-mmarkdown-mmark".
  3. [ ] Eventually package the new golang-github-mmarkdown-mmark (unless someone else beats me to it.)

@dkg there is no mmark v2. The mmark that Hugo uses is stuck in v1. There is a completely new and totally different package with a different name in which Debian should feel free to use if they want.

Amen! What bep wrote is entirely true. For the mmark situation to be solved in Debian, you'll just need to reach the right people who find time to do it, and a little bit of luck. ;-)

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.