atom / markdown-preview

📝 Markdown preview in Atom
MIT License
1.23k stars 357 forks source link

Stop using roaster for rendering markdown #559

Closed rafeca closed 5 years ago

rafeca commented 5 years ago

Requirements

Description of the Change

In order to fix https://github.com/atom/markdown-preview/issues/206, the version of the marked package needed to be upgraded. Since this package depended on roaster, which is unmaintained and not even recommended to be used by its own creator, I've opted to replace it by directly calling marked and implementing in this package the missing functionality.

Alternate Designs

I could have forked roaster (in fact a really old fork of it already exists on the atom org: http://github.com/atom/roaster) and then upgrade marked from that fork, but I prefer to avoid having yet another fork and yet another package to maintain.

Benefits

Fixes https://github.com/atom/markdown-preview/issues/206

Possible Drawbacks

N/A

Applicable Issues

https://github.com/atom/markdown-preview/issues/206

rafeca commented 5 years ago

In order to verify that the extension keeps working correctly, I've used the following markdown file:

https://gist.github.com/rafeca/7220f08c6d7d340533b3c323290ac507

Using standard style:

Screen Shot 2019-04-23 at 20 37 47

Using GitHub style:

Screen Shot 2019-04-23 at 20 38 08
rafeca commented 5 years ago

If for some reason someone were to use an emoji string inside a URL, it would break rendering. Like this: :finnadie:.

Good point! yeah this whole emoji thing is quite fragile (the logic in this PR mimics the one that we had before with roaster).

I wonder if we could get rid of the :colonemoji: handling for Markdown files (which also adds ~5.5MB to Atom due to all the emoji images being included in the app via the emoji-images packages), but I guess that this is such a distinctive GitHub feature that it's hard to justify removing it.