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

What's the point of the markdown filter? #2938

Open
1 task done
jvoisin opened this issue Nov 12, 2024 · 6 comments
Open
1 task done

What's the point of the markdown filter? #2938

jvoisin opened this issue Nov 12, 2024 · 6 comments
Labels

Comments

@jvoisin
Copy link
Contributor

jvoisin commented Nov 12, 2024

Miniflux supports converting feeds from markdown to html via github.com/yuin/goldmark, but I fail to see a usecase, as I don't know a single rss/atom feed that spits markdown and not html.

If it has no use, it' be nice to remove it, as reducing the amount of external dependencies is almost always a win, packaging-wise, security-wise, reliability-wise… but also size-wise, as it adds adds a lot of code to the miniflux binary:

$ goweight | grep goldmark
  2.5 MB github.com/yuin/goldmark/ast
  2.4 MB github.com/yuin/goldmark/util
  1.7 MB github.com/yuin/goldmark/parser
  448 kB github.com/yuin/goldmark/renderer/html
  364 kB github.com/yuin/goldmark/text
  139 kB github.com/yuin/goldmark
  114 kB github.com/yuin/goldmark/renderer
$
@fguillot
Copy link
Member

It was introduced in this PR #1513

This PR adds a new rewrite rule that parses markdown. Parsing is done with a new dependency yuin/goldmark. This PR also configures the parse markdown rule by default for blog.laravel.com.

Removing this feature would be a breaking change for the few people that use it. I'm not aware of any lightweight alternative Markdown parser library in Go.

@jvoisin
Copy link
Contributor Author

jvoisin commented Nov 13, 2024

I don't think it'll break anything, as nobody should be using it. The reason it was added, blog.laravel.com, doesn't publish markdown.

@gabe565
Copy link
Contributor

gabe565 commented Nov 22, 2024

@jvoisin Funny timing, I actually just opened a PR to remove that rewrite rule (#2958). When I originally added this filter, Laravel's feed was in markdown, but they've since fixed it. It looks like there's another rewrite rule for https://www.recalbox.com/rss.xml which still returns markdown. It isn't very common (and honestly probably a feed generation bug), but it seems to happen occasionally. That being said, I agree that a smaller binary would be nice...

Edit: In actuality, it adds ~2MiB to the binary size

-rwxr-xr-x@  1 gabe565  gabe565    20M Nov 22 01:11 miniflux-with-markdown
-rwxr-xr-x@  1 gabe565  gabe565    18M Nov 22 01:12 miniflux-without-markdown

@jvoisin
Copy link
Contributor Author

jvoisin commented Nov 22, 2024

I don't think that any other feed reader does support markdown. It does indeed looks like a feed generation bug to me.

@gabe565
Copy link
Contributor

gabe565 commented Nov 22, 2024

True! Although in my experience, most feed readers don't support rewrite rules in general.

I just tested recalbox with "Fetch original content" and without parse_markdown. Posts loaded fine, so it isn't necessarily required for that one.

@jvoisin
Copy link
Contributor Author

jvoisin commented Nov 22, 2024

@fguillot what do you think? If I sent a pull-request to remove markdown support, will it get considered/merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants