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

feature idea: favicons #245

Closed
chrisgrieser opened this issue Nov 30, 2024 · 7 comments
Closed

feature idea: favicons #245

chrisgrieser opened this issue Nov 30, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@chrisgrieser
Copy link
Contributor

Is your feature request related to a problem? Please describe.

This is just a basic idea, but when using Obsidian, I really enjoyed using the favicon plugin. I was wondering whether render-markdown could maybe achieve a similar thing?

Describe the solution you'd like

fetching and displaying favicons inline could be tricky, but it appears that there are nerdfont icons for a lot of common websites (github, stackoverflow, slack, etc.). Just handling them should already cover a large number of links. The favicon could replace 🌐, and the globe icon is only used as fallback if the site does not have a nerdfont favicon.

Describe alternatives you've considered

not aware of any.

Additional information

No response

@chrisgrieser chrisgrieser added the enhancement New feature or request label Nov 30, 2024
@MeanderingProgrammer
Copy link
Owner

Links currently support custom lua patterns on the destination that let you use different icons under link.custom: https://github.com/MeanderingProgrammer/render-markdown.nvim?tab=readme-ov-file#links. The globe uses this for any http URL. You can add others for specific domains like youtube for example:

require('render-markdown').setup({
    link = {
        custom = {
            youtube = { pattern = 'www%.youtube%.com/', icon = '', highlight = 'RenderMarkdownLink' },
        },
    },
})

Is this the feature you're looking for? If you have suggestions for more out of the box domains (which I think is what you're getting at) LMK and I can expand the default configuration.

@chrisgrieser
Copy link
Contributor Author

chrisgrieser commented Dec 1, 2024

ah, yes indeed, seems I have missed that, my bad.

And indeed, expanding some defaults would be a nice improvement.


However, it seems there is also some bug with the feature – when using this config (which I also would suggest as default):

link = {
	custom = {
		-- stylua: ignore start
		youtube = { pattern = "youtube%.com", icon = "", highlight = "RenderMarkdownLink" },
		github = { pattern = "github%.com", icon = "", highlight = "RenderMarkdownLink" },
		neovim = { pattern = "neovim%.io", icon = "", highlight = "RenderMarkdownLink" },
		stackoverflow = { pattern = "stackoverflow%.com", icon = "󰓌 ", highlight = "RenderMarkdownLink" },
		discord = { pattern = "discord%.com", icon = "󰙯 ", highlight = "RenderMarkdownLink" },
		reddit = { pattern = "reddit%.com", icon = "", highlight = "RenderMarkdownLink" },
		-- stylua: ignore end
	},
},

and this markdown:

## Some links
- [GitHub](https://github.com/chrisgrieser)
- [YouTube](https://www.youtube.com/aaaaaa)
- [Stackoverflow](https://stackoverflow.com/fsfsf)
- [Neovim Docs](https://neovim.io/doc/user/treesitter.html)
- [Discord](https://discord.com/channels/686053708261228577/1309959771863908532)
- [reddit](https://www.reddit.com/r/neovim/comments/1h43mjj/snacksprofiler_a_neovim_lua_profiler/)

Only 2 out of 6 get the correct icon:
Showcase

Weirdly enough, using pattern = "youtube.com" instead of pattern = "youtube%.com" breaks the YouTube icon, even though . and %. in a lua pattern should both match a ., so I guess there is some issue with how the patterns are processed?

MeanderingProgrammer added a commit that referenced this issue Dec 2, 2024
## Details

Request: #245

Adds support for several URLs out of the box in addition to default web:

- youtube.com
- github.com
- neovim.io
- stackoverflow.com
- discord.com
- reddit.com

2 other changes have been made to make configuring custom links easier:

- The highlight gets pulled from the top level `link.highlight` option
  if a link specific one is not specified. This makes it so you don't
  need to copy `RenderMarkownLink` repeatedly while being backwards
  compatible and allowing users to override behavior.
- When multiple patterns match a link we break ties by picking the
  longer pattern, idea being more specific pattern = longer pattern.
  However patterns can be long for other reason like the original
  default web pattern `^http[s]?://`, to make overriding this value more
  likely the pattern has been shortened to `^http`.
@MeanderingProgrammer
Copy link
Owner

Added these here: 61850bf, thanks!

Mostly the same as what you have, but I did change some of the icons to nf-md-* versions when they were available for internal consistency.

When multiple patterns match a custom link destination, like with https://www.youtube.com/aaaaaa both youtube.com and ^http[s]?:// match, we pick the pattern that is longer: https://github.com/MeanderingProgrammer/render-markdown.nvim/blob/main/lua/render-markdown/render/base.lua#L71-L73.

This is to make the behavior more likely to be consistent and will probably do the right thing in most cases, since longer patterns tend to be more specific. But in this case the shortened form of the youtube pattern is shorter than the base web pattern, as is the case for some of the other URLs. I went ahead and updated the default web pattern to the simpler ^http, which should function mostly the same as before but make it more likely to be overridden by other destinations.

@chrisgrieser
Copy link
Contributor Author

Thank you!

When multiple patterns match a custom link destination, like with https://www.youtube.com/aaaaaa both youtube.com and ^http[s]?:// match, we pick the pattern that is longer

Just a minor nit: May I suggest changing this behavior, e.g. by using a priority field or by using the alphabetical order of the key? The length of a pattern feels off, resulting in unexpected behavior like I mentioned above with the %. instead of . changing behavior.

In any case, I think this issue can be closed, since it has been addressed. :)

@MeanderingProgrammer
Copy link
Owner

I considered adding a more explicit priority field, but I think surfacing this behavior to users would be tricky, more of a consequence of how large the configuration is.

I liked the idea of having things work how you would expect out of the box. Like if you specify overlapping patterns you probably want the one with more overlap to be used, definitely some edge cases with this approach.

If it comes up as a problem again definitely LMK!

@chrisgrieser
Copy link
Contributor Author

chrisgrieser commented Dec 3, 2024

I liked the idea of having things work how you would expect out of the box.

Well, as I was trying to say, the current way does not work how I expected out of the box, with the pattern behavior appearing buggy at first.

Like if you specify overlapping patterns you probably want the one with more overlap to be used, definitely some edge cases with this approach.

In general, this is actually reasonable, yeah. Maybe simply documenting this behavior in a short one-liner might be helpful, so that there is no confusion like I had?

@MeanderingProgrammer
Copy link
Owner

Makes sense, added documentation: 8790a38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants