feat(api): get_icons_by_xxx

[ibhagwan]

The current get_icons API only allows getting the merged icon set, this PR exposes the internal icons_by_filename and icons_by_file_extension.

You may ask why is this required or how is this used, for performance reasons, fzf-lua extracts the icon sets directly from nvim-web-devicons and uses its own logic to perform the lookup (by filename, 2-part extension, extension, etc), up until now I would extract the internal structures directly from the theme files nvim-web-devicons.icons-light, nvim-web-devicons.icons-default but this has a limitation of not including new extensions coming from user overrides.

Context: https://github.com/ibhagwan/fzf-lua/issues/1152

[ibhagwan]

Nice optimisation. We should use this in nvim-tree; the repeated API calls bother me.

The logic of the optimized query is explained here: https://github.com/ibhagwan/fzf-lua/pull/1053#issuecomment-2002302681

The implementation is here: https://github.com/ibhagwan/fzf-lua/blob/2a1bb74da8e409d8c534a8f43d7fb0f8acc85207/lua/fzf-lua/devicons.lua#L143-L195

Not sure if I understand your review request, would you like me to add additional API interfaces to expose icons_by_operating_system, etc as well as mention that in the README?

[gegoune]

Since this would be new API, it's a good time to think little bit about naming. I am not saying there is anything wrong with names mentioned by both of you above, but perhaps we can come up with something else/better?

[ibhagwan]

Since this would be new API, it's a good time to think little bit about naming. I am not saying there is anything wrong with names mentioned by both of you above, but perhaps we can come up with something else/better?

I don’t mind at all but this isn’t my package, I was merely following the naming conventions of the already existing variables.

[alex-courtis]

Since this would be new API, it's a good time to think little bit about naming. I am not saying there is anything wrong with names mentioned by both of you above, but perhaps we can come up with something else/better?

I don’t mind at all but this isn’t my package, I was merely following the naming conventions of the already existing variables.

It does follow get_icons. It does follow get_icon_by_filetype, even though they are not quite the same usage.

It doesn't match the setup override's override_by_extension instead using the private name is _by_file_extension. Let's change that to get_icons_by_extension please.

[alex-courtis]

Not sure if I understand your review request, would you like me to add additional API interfaces to expose icons_by_operating_system, etc as well as mention that in the README?

Yes please. README doesn't have to be fancy, maybe just:

### Get all icons

It is possible to get all of the registered icons with the `get_icons()` function:

require'nvim-web-devicons'.get_icons()

This can be useful for debugging purposes or for creating custom highlights for each icon.

Mapped categories can be fetched via:

require'nvim-web-devicons'.get_icons_by_filename()
require'nvim-web-devicons'.get_icons_by_extension()
...
...

[ibhagwan]

@alex-courtis, https://github.com/nvim-tree/nvim-web-devicons/pull/450/commits/977e4dfd261a8a5325b6ef151982cd71eab6b66c.

[alex-courtis]

Added OS, DE and WM.

@gegoune are you happy with the naming?

We really need to doc/help this thing properly...

[ibhagwan]

@alex-courtis, as part of my testing I discovered a flaw in the logic, consider the below setup which adds a previously unknown extension for norg:

require("nvim-web-devicons").setup({
  override_by_extension = {
    norg = {
      icon = "",
      color = "#97eefc",
      name = "Neorg",
    },
  },
})

If the backgound changes from dark to light, the refresh autocmd will be executed in https://github.com/nvim-tree/nvim-web-devicons/blob/4f499a0e8cd64faa29b89446bdadde070dc0d397/lua/nvim-web-devicons.lua#L565-L569

This, in turn, will call refresh_icons which will then deletes the user overrides in: https://github.com/nvim-tree/nvim-web-devicons/blob/4f499a0e8cd64faa29b89446bdadde070dc0d397/lua/nvim-web-devicons.lua#L35

The user_icons is a local variable in the setup scope only, any adittional refresh calls deletes any customizations, which will then return the wrong result in get_icon_by_extension , eventually creating a bug in get_icon.

[alex-courtis]

The user_icons is a local variable in the setup scope only, any adittional refresh calls deletes any customizations, which will then return the wrong result in get_icon_by_extension , eventually creating a bug in get_icon.

That's not a great problem; the overrides are all persisted in global_opts.override and injected into icons during setup and refresh. See hi-test which unpacks the global overrides.

Options:

1. New method to retrieve overrides
2. 5 get methods merge icons_by_XXX and global_opts.override.XXX

1 is not nice for the user; 2 is simple

[ibhagwan]

The user_icons is a local variable in the setup scope only, any adittional refresh calls deletes any customizations, which will then return the wrong result in get_icon_by_extension , eventually creating a bug in get_icon.

That's not a great problem; the overrides are all persisted in global_opts.override and injected into icons during setup and refresh. See hi-test which unpacks the global overrides.

Options:

1. New method to retrieve overrides
2. 5 get methods merge icons_by_XXX and global_opts.override.XXX

1 is not nice for the user; 2 is simple

It is not a big deal indeed but it is a bit inconsistent.

1. Retrieves all overrides, I cannot tell from the merged table whether this was an extension or filename override.
2. Similar issue to (1), due to global_opts.override being a merged table I will have to merge all overrides into every icons_by_xxx table, you might say we can merge only already existing values but then I will lose new extensions that were added in the overrides as the norg example I posts above

Overall not a big issue as not too many users have complex icon overrides and play around with neovim’s background but it is still a potential flaw in the logic, if not now, then sometime in the future (if you decide to optimize the lookups similar to what I linked).

[alex-courtis]

Fair enough. There are some fundamental issues in web-devicons.

Unit tests are needed before we can refactor...

I'm happy if @gegoune is happy.

[ibhagwan]

Tysm @alex-courtis!