Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

markdown: Pass emoji codes to yuin/goldmark-emoji #11593

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

jmooring
Copy link
Member

Fixes #7332
Fixes #11587

@jmooring jmooring marked this pull request as draft October 22, 2023 18:05
@jmooring
Copy link
Member Author

jmooring commented Oct 22, 2023

So, this works great, but right now you need to do two things in site config to use it:

enableEmoji = false
[markup.goldmark.extensions]
emoji = true # default is false

It would be better if enableEmoji were the primary on/off switch, with goldmark/emoji enabled by default. The problem I'm having is in the page parser and shortcode parser:

(

hugo/hugolib/page.go

Lines 796 to 801 in de4e466

case it.Type == pageparser.TypeEmoji:
if emoji := helpers.Emoji(it.ValStr(result.Input())); emoji != nil {
rn.AddReplacement(emoji, it)
} else {
rn.AddBytes(it)
}
)

What I want to do is:

if this_content_will_be_rendered_by_goldmark {
  rn.AddBytes(it) 
} else {
  if emoji := helpers.Emoji(it.ValStr(result.Input())); emoji != nil { 
    rn.AddReplacement(emoji, it) 
  } else { 
    rn.AddBytes(it) 
  } 
}

But I can't figure out how to determine this_content_will_be_rendered_by_goldmark.

@bep
Copy link
Member

bep commented Oct 22, 2023

But I can't figure out how to determine this_content_will_be_rendered_by_goldmark.

So, the Emoji feature was added a long time ago. I'm tempted to make that into a Goldmark only thing. We need to reduce some of the complexity in Hugo, and, as the issues referenced above, what we have isn't really working so we might as well just cut it.

We should translate enableEmoji to the new Goldmark option.

@jmooring
Copy link
Member Author

Goldmark only thing

I'm fine with that, as long as we understand that existing emoji in org, pandoc, rst, asciidoc, and html will no longer render (breaking change). I just tested the first four to verify that the renders don't already have this baked in.

I'll update the PR.

Removes emoji code conversion from the page and shortcode parsers. Emoji
codes in markdown are now passed to Goldmark, where the goldmark-emoji
extension converts them to decimal numeric character references.

This disables emoji rendering for the alternate content formats: html,
asciidoc, org, pandoc, and rst.

Fixes gohugoio#7332
Fixes gohugoio#11587
@jmooring jmooring force-pushed the feat/goldmark-emoji branch from 8fc55c3 to 726f3ec Compare October 23, 2023 02:19
@jmooring jmooring changed the title markdown: Add yuin/goldmark-emoji markdown: Pass emoji codes to yuin/goldmark-emoji Oct 23, 2023
@jmooring
Copy link
Member Author

jmooring commented Oct 23, 2023

enableEmoji in site config now enables/disables the yuin/goldmark-emoji extension. No other config setting required. For markdown users, there is one expected change that is covered in the test cases:

{{< sc >}}:smiley:{{< /sc >}}

The shortcode must now do this:

{{ .Inner | .Page.RenderString }}

Instead of this:

{{ .Inner }}

Shortcodes called with the {{% sc %}} notation continue to work as they did before.


And it is a bit faster. With a 500 page site with 1877 emoji on each page, yuin/goldmark-emoji reduces the build time by 20%. But that's pretty much noise, because site builds in about a second.

@jmooring jmooring marked this pull request as ready for review October 23, 2023 02:28
T: t,
TxtarString: files,
},
).Build()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A tip: I found that I repeated the above construct so often, so I added 2 convenience funcs named Test and TestRunning which you could rewrite the above to:

b := hugolib.Test(t, files)

@bep bep merged commit 272484f into gohugoio:master Oct 24, 2023
7 checks passed
@jmooring jmooring deleted the feat/goldmark-emoji branch October 24, 2023 13:39
@jmooring jmooring mentioned this pull request Nov 15, 2023
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants