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

Beta-testing 'mini.icons' #1007

Closed
Tracked by #3899
echasnovski opened this issue Jul 3, 2024 · 119 comments
Closed
Tracked by #3899

Beta-testing 'mini.icons' #1007

echasnovski opened this issue Jul 3, 2024 · 119 comments

Comments

@echasnovski
Copy link
Owner

Please leave your feedback about new mini.icons module here. Feel free to either add new comment or positively upvote existing one.

Some things I am interested to find out (obviously, besides bugs):

  • Does it work well out of the box with your existing setup? Like:

    • Are icons rendered well? If not, there might be a terminal emulator and/or font issue, see Dependencies section.
    • Are colors for icons that are important for you chosen reasonably?
  • Do you have any suggestion about changing some icon's glyph and/or highlight group? I'd like to warn right away about what probably won't be done:

    • Adding narrowly scoped programming language-related icons. For example: special treatment of d.ts extension.
    • Changing glyph which is explicitly designed for where it is used. For example: almost whole "lsp" category.

    What I am open for and welcome:

    • Adjusting filetype glyph which was added from a set of "generic" nf-md-alpha-* icons. For example, like this. It needs a reasonable suggestion, though.
    • Adding new filetype icon data for plugins, if they have workflow which allows making buffer with that filetype current (like opening floating window or split with the buffer). Most of popular ones are already added, but I might have missed some.
  • Is configuration intuitive enough?

  • Are default values of settings convenient for you?

  • Is documentation and examples clear enough?

Thanks!

@SebasF1349
Copy link

Great new module! I've been playing with it and doing some comparations with devicons and I found out that icons are missing in files with "double" extensions. Specially in typescript files (javascript is working ok). I can't think of any other language I use that have those.

Here is a pic from telescope (that works great with the mocking).

wezterm-gui_1GrTuDXBP9

Still not sure about some colors, will have to think about it as I'm not sure if I think they are wrong or it's just that I have to get use to them. Will post later about it.

@folke
Copy link
Contributor

folke commented Jul 3, 2024

First of all, thank you for this great new module!

Just wanted to let you know it currently doesn't work with fzf-lua.
It trips at the line https://github.com/ibhagwan/fzf-lua/blob/3b91c1a471160bd8620bdca8f18743d954994daa/lua/fzf-lua/devicons.lua#L81

@folke
Copy link
Contributor

folke commented Jul 3, 2024

You should probably also change this line https://github.com/echasnovski/mini.icons/blob/b5e40acb2f0de7127bbcf026f3a0a55189a428a4/lua/mini/icons.lua#L427

When lua starts loading a module, it changes the value in package.loaded to true.
So better to check for type == "table".

@folke

This comment was marked as resolved.

@echasnovski
Copy link
Owner Author

I found out that icons are missing in files with "double" extensions. Specially in typescript files (javascript is working ok). I can't think of any other language I use that have those.

@SebasF1349, thanks for the info! I am mostly positive that it is a typescript issue and not a "double extension" one. The problem here is that built-in vim.filetype.match() doesn't currently recognize 'ts' extension at all when used outside of some buffer context.

This means that telescope uses 'nvim-web-devicons' as get_icon(nil, 'd.ts', {}) instead of get_icon('app.d.ts', nil, {}) (i.e. supplies extension directly instead of file name.

You can check that 'mini.icons' does work correctly via :=MiniIcons.get('file', 'app.d.ts') which should return icon for typescript and "MiniIconsAzure" highlight group.


That said, during writing this I've got an idea of quite an easy fix for this in 'mini.icons'. Needs a bit more profiling but I think it will be solved.


So better to check for type == "table".

Work-around to make it work on fzf-lua

@folke, thanks for early beta-testing! I thought that mocking all get_icons_*() is meaningless as did not see the use for it. Seems like it will have to be done (at least so that you don't have to have it in LazyVim :) ). As it needs testing, I hope to do this tomorrow.

@folke
Copy link
Contributor

folke commented Jul 3, 2024

About that ts thing, I had to include work-arounds for that in a number of my plugins.
The reason is that the ft detector tries to read the buffer (which is nil) and will error. That in turn will completely stop the ft check unfortunately.

The offending line in filetypes.lua is ts = detect_line1('<%?xml', 'xml', 'typescript').

Easiest way to fix is probably to always have a hidden scratch buffer ready that can be used as an argument.

And thanks for adding that extra stuff for nvim-web-devicons!

@echasnovski
Copy link
Owner Author

About that ts thing, I had to include work-arounds for that in a number of my plugins.
The reason is that the ft detector tries to read the buffer (which is nil) and will error. That in turn will completely stop the ft check unfortunately.

Yes, I figured this also some time ago. I planned to make fix this upstream by also recognizing contents properly instead of buf.

Easiest way to fix is probably to always have a hidden scratch buffer ready that can be used as an argument.

That was exactly what I thought about during writing that comment. Don't know why I didn't think about this sooner 🤦

@0x0003
Copy link

0x0003 commented Jul 3, 2024

extension field doesn't seem to have any effect on dotfiles. E.g. :

require('mini.icons').setup({
  extension = {
    ['ghci'] = {
      glyph = '󰅩',
      hl = 'MiniIconsPurple'
    },
  }
})

Results in:
Untitled

EDIT:
Scratch that, there's a file field for this case. Should've read the docs more thoroughly instead of skimming through 😕

@wroyca
Copy link
Contributor

wroyca commented Jul 4, 2024

Thank for this! Testing it right now.

Are icons rendered well? If not, there might be a terminal emulator and/or font issue, see Dependencies section.

Note for those using Kitty; make sure to update your symbol_map here: https://sw.kovidgoyal.net/kitty/faq/#kitty-is-not-able-to-use-my-favorite-font


Changing glyph which is explicitly designed for where it is used. For example: almost whole "lsp" category.

Adjusting filetype glyph which was added from a set of "generic" nf-md-alpha-* icons. For example, like this. It needs a reasonable suggestion, though.

After using it today, here are some personal observations that might offer a different perspective;

Directory icons:

Enforcing "folder" type icons for all results in a mixed set. Personally, I believe that allowing exceptions for directories such as .github and .git contributes to better overall visual consistency:

- ['.git']      = { glyph = '', hl = 'MiniIconsOrange' },
- ['.github']   = { glyph = '', hl = 'MiniIconsAzure'  },
+ ['.git']      = { glyph = '󰊢', hl = 'MiniIconsOrange' },
+ ['.github']   = { glyph = '󰊤', hl = 'MiniIconsAzure'  },
Software:

The "executable" icon really stands out from the other "outline" icons in the same category. I think nf-oct-gear would fit better overall here (nf-md-cog-outline is a little too thick to blend well with the rest):

-exe  = { glyph = '󰒔', hl = 'MiniIconsGrey'  },
+exe  = { glyph = '', hl = 'MiniIconsGrey'  },
File icons:

The README icon stands out too much. Something like nf-md-file_document or nf-md-file_document_alert would, I believe, be more fitting:

-README = { glyph = '', hl = 'MiniIconsYellow' },
+README = { glyph = '󰈙', hl = 'MiniIconsYellow' },
-['README.md'] = { glyph = '', hl = 'MiniIconsYellow' },
+['README.md'] = { glyph = '󰈙', hl = 'MiniIconsYellow' },
Everything else:

Skipped, as it seems too specific to warrant any significant changes, and it already looks quite good to me as it is :)


Will continue to use it for the time being 🥳

@xarvex
Copy link

xarvex commented Jul 4, 2024

Thanks for yet another amazing module! I'm loving the style out of the box enough that I haven't really messed with any settings just yet. I have it working in oil currently. That being said, for some odd reason I am slightly different icons in Telescope (but they also aren't the same as nvim-web-devicons), I have attached a screenshot:
Screenshot from 2024-07-03 22-46-54

@wroyca
Copy link
Contributor

wroyca commented Jul 4, 2024

Consider adding a Projects directory icon; the proposal is ongoing: https://gitlab.freedesktop.org/xdg/xdg-user-dirs/-/issues/3 but it'd be nice for those already using it :)

image

@echasnovski
Copy link
Owner Author

I've pushed some changes after early beta-testing.


There is now better fallback to vim.filetype.match(). @SebasF1349, this should fix the "missing ts icons in Telescope" problem (I've checked).


There is now fuller and (hopefully) smarter mocking in mock_nvim_web_devicons(). @folke, I think the whole LazyVim/LazyVim#3899 workarounds are now not needed. At least, I've tested and 'ibhagwan/fzf-lua' works with mocks. Although first class 'mini.icons' support should come to it eventually.


@wroyca, thanks for detailed feedback. Highly appreciated! Here are some thoughts:

  • I have very strong opinion that directory icons should all resemble same "folder" icon. This is the main way to differentiate files and directories. Especially considering that '.git' can be both file and directory.
  • I understand the point of "README icon stands out too much", but standing out was the intention here. Especially as it is shown in demo, it will be hard to convince me to change it :)
  • Agree about "exe" icon. I replaced it with "windows" icon (as I saw LazyGit uses it) with red color (that draws attention and is not blue used in "os").
  • I'll gladly add "Projects" directory after it is made official. I don't think it is used universally enough to be included now.

@xarvex, the reason why you see this seems to be because in floating window require('nvim-web-devicons').get_icon() is used with extension and not as the file name. While 'oil.nvim' seems to use file name. Those are mocked with "extension" and "file" categories respectively which act slightly differently. The best suggestion here is to wait for the first class 'mini.icons' support.

folke added a commit to LazyVim/LazyVim that referenced this issue Jul 4, 2024
## What is this PR for?

Replace the icon support with the new mini library

## Blockers

- [ ]
echasnovski/mini.nvim#1007 (comment)
@folke
Copy link
Contributor

folke commented Jul 4, 2024

Sweet! Just merged my PR to use mini.icons in LazyVim :)

@echasnovski
Copy link
Owner Author

Sweet! Just merged my PR to use mini.icons in LazyVim :)

Very noice! Expecting bazillion issues about it now :)


Is there a reason you still use package.preload["nvim-web-devicons"]? Because 'mini.icons' now sets it itself. I hoped this would result in you not needing it in LazyVim.

@folke
Copy link
Contributor

folke commented Jul 4, 2024

That's just to lazyload mini.icons.
mini.icons will only be loaded (and setup) when either mini.icons or nvim-web-devicons is required

@folke
Copy link
Contributor

folke commented Jul 4, 2024

For your use-case you should probably not use preload, since you're executing the code anyway, so there's no benefit and you can just define the nvim-web-devicons module.

@echasnovski
Copy link
Owner Author

echasnovski commented Jul 4, 2024

Hm, OK. I would have assumed that this would conflict with how mocking is now done. But if it works, then fine.

I don't think there is more 'mini.icons' can do here to make it easier for this type of mocking on user side, right?

For your use-case you should probably not use preload, since you're executing the code anyway, so there's no benefit and you can just define the nvim-web-devicons module.

Oh, well. It's already there, so let it be until it causes some issue.

@folke
Copy link
Contributor

folke commented Jul 4, 2024

preload is only useful when:

  • you want to define a module that is not a real module
  • and you want to defer the cost of creating the module to the point where the module is initially required

So for LazyVim it is useful, but not inside mini.icons, since the cost of defining the module in your case is currently return M. You could put that whole local M = {} .... inside the preload, but the benefit in terms of loading would be practically nothing either, so not worth it.

Either way, that if statement at the end, indeed makes sure this won't lead to issues, so all good :)

@SebasF1349
Copy link

Confirming the issue with TS is gone. Thanks!

But now I've found another icon missing, again with Telescope:

wezterm-gui_QxqJ5HEC21

The same happens with .npmignore and apparently all icons in H.file_icons. For example, README.md shows a md icon and init.lua shows a lua icon.

After looking at the code, I see you are prioritazing extension over file, so the result makes sense. Is this a design choice?

@echasnovski
Copy link
Owner Author

echasnovski commented Jul 4, 2024

But now I've found another icon missing, again with Telescope:

This is because Telescope calls get_icon() with both filename and (manually computed based on some custom logic) extension. In this case the original get_icon() first checks exact filename match, then prefers extension until finally computing extension from file name. I might add the first part of preferring exact file match, but the second one I'd not add to mock. I think the most proper way to original get_icon() in this scenario is to supply only filename.

@Yushi5058
Copy link

Hello, is this normal ? mini.icons not working for me.
I just added require("mini.icons").setup() like the other mini.plugins
Output :
2024-07-04_20-43-45
Arch Linux 6.6.36-1-lts - NVIM v 0.10.0 - LuaJIT 2.1.1713484068

Thanks

@echasnovski
Copy link
Owner Author

echasnovski commented Jul 4, 2024

Hello, is this normal ? mini.icons not working for me.
I just added require("mini.icons").setup() like the other mini.plugins

Hi! No, this is not normal. It looks like you haven't updated 'mini.nvim' to latest main branch. Your dotfiles point at latest stable 0.13.0 release.

@mehalter
Copy link
Contributor

mehalter commented Jul 4, 2024

It's awesome to see this plugin and the "mock" nvim-web-devicons. Another plugin that is quite popular in the community is lspkind-nvim which is used in many plugins as well. Would it be beneficial to have a mock setup for lspkind as well which would help users use mini.icons in more places such as LSP symbol browsers, their nvim-cmp formats, etc.

@Yushi5058
Copy link

Hello, is this normal ? mini.icons not working for me.
I just added require("mini.icons").setup() like the other mini.plugins

Hi! No, this is not normal. It looks like you haven't updated 'mini.nvim' to latest main branch. Your dotfiles point at latest stable 0.13.0 release.

Thanks, it's working !

@Allaman
Copy link

Allaman commented Jul 4, 2024

Hello, some recent changes broke lualine. When I pin mini.icons to b5e40acb2f0de7127bbcf026f3a0a55189a428a4 from yesterday, there is no such error.

For reference, here is my commit with the changes I made and the (working) pinned mini.icon

edit: This is the not working commit: 3c51394b8815e0cb9beb385909803cd4816951c4

version

   Error  22:51:21 msg_show.lua_error Error executing vim.schedule lua callback: ...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:429: Error executing lua: ...im/lazy/lualine.nvim/lua/lualine/components/filetype.lua:34: attempt to call field 'get_icon' (a nil value)
stack traceback:
	...im/lazy/lualine.nvim/lua/lualine/components/filetype.lua:34: in function 'apply_icon'
	...l/share/nvim/lazy/lualine.nvim/lua/lualine/component.lua:280: in function 'draw'
	...are/nvim/lazy/lualine.nvim/lua/lualine/utils/section.lua:26: in function 'draw_section'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:170: in function 'statusline'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:298: in function <...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:279>
	[C]: in function 'nvim_win_call'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:429: in function 'refresh'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:353: in function <...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:352>
stack traceback:
	[C]: in function 'nvim_win_call'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:429: in function 'refresh'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:353: in function <...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:352>
   Error  22:51:25 msg_show.lua_error Error executing vim.schedule lua callback: ...share/nvim/lazy/lualine.nvim/lua/lualine/utils/utils.lua:211: lualine: Failed to refresh statusline:
...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:429: Error executing lua: ...im/lazy/lualine.nvim/lua/lualine/components/filetype.lua:34: attempt to call field 'get_icon' (a nil value)
stack traceback:
	...im/lazy/lualine.nvim/lua/lualine/components/filetype.lua:34: in function 'apply_icon'
	...l/share/nvim/lazy/lualine.nvim/lua/lualine/component.lua:280: in function 'draw'
	...are/nvim/lazy/lualine.nvim/lua/lualine/utils/section.lua:26: in function 'draw_section'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:170: in function 'statusline'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:298: in function <...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:279>
	[C]: in function 'nvim_win_call'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:429: in function 'refresh'
	...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:524: in function <...hael/.local/share/nvim/lazy/lualine.nvim/lua/lualine.lua:523>
	[C]: in function 'pcall'
	...share/nvim/lazy/lualine.nvim/lua/lualine/utils/utils.lua:214: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>
stack traceback:
	[C]: in function 'error'
	...share/nvim/lazy/lualine.nvim/lua/lualine/utils/utils.lua:211: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>

@Joao-Queiroga
Copy link

Joao-Queiroga commented Jul 4, 2024

Is there a way to add an icon and a HEX code in the highlight?

@wenjinnn
Copy link

wenjinnn commented Jul 5, 2024

It's awesome to see this plugin and the "mock" nvim-web-devicons. Another plugin that is quite popular in the community is lspkind-nvim which is used in many plugins as well. Would it be beneficial to have a mock setup for lspkind as well which would help users use mini.icons in more places such as LSP symbol browsers, their nvim-cmp formats, etc.看到这个插件和“模拟”nvim-web-devicons 真是太棒了。另一个在社区中非常流行的插件是 lspkind-nvim ,它也被用于许多插件中。对 lspkind 进行模拟设置是否有益,这将有助于用户在更多地方使用 mini.icons ,例如 LSP 符号浏览器及其 nvim-cmp 格式, ETC。

About mocking lspkind-nvim in nvim-cmp, you can simply use the following code to achieve the same effect:

    cmp.setup({
      formatting = {
        format = function(entry, vim_item)
          -- replace kind with mini lsp icon
          local icon, _ = require("mini.icons").get("lsp", vim_item.kind)
          if icon ~= nil then
            vim_item.kind = icon
          end
          return vim_item
        end,
      },
    })

@echasnovski
Copy link
Owner Author

@echasnovski why my inactive window can not display the icon, like this

I am pretty sure it is not the problem with 'mini.icons' per se. What plugin is responsible for showing those icons and which icon-highlight pair do you expect to see there?

@yifan0414
Copy link

@echasnovski why my inactive window can not display the icon, like this

I am pretty sure it is not the problem with 'mini.icons' per se. What plugin is responsible for showing those icons and which icon-highlight pair do you expect to see there?

thanks, it's lualine's problem

@wfpaisa
Copy link

wfpaisa commented Jul 21, 2024

hello, it would be nice if you could add an optional gap between the icon and the text.

I don't think this is s good idea. Adding on MiniIcons.get() level solves a minor problem because caller can manually add space afterwards pretty easily (that's what is done in all 'mini.nvim' usages). While adding this on user config level will result in inconsistent behavior (as presumably some plugins add space and some don't).

By the way, judging by the screenshot it looks like there is a space afterwards which terminal emulator uses to show wider glyph. You can try disabling this behavior on terminal emulator (or maybe possibly even font) side.

Hello echasnovski, thank you for responding, I am very grateful for your plugin.

I tried validating what you told me, the terminal in kitty, alacrity, blackbox, and iTerm, and vscode looks consistent with the glyphs. I tried changing the font that I created and using three of the most common ones, and it looks the same. So I thought it's not a problem with the glyphs because they overflow into the next cells. So it's not a configuration issue. If I disable the ability to extend the glyphs, the rest of the configurations will look bad. So if I just want to leave a space or add a hyphen, etc., I have to go through all the icons, adding a space. Could you please indicate how I can do this as you mentioned with: "MiniIcons.get()".

At the moment, I tried applying by overwriting and customizing the ones I use most frequently, but it is a one-by-one task.
Pasted image
Pasted image (2)

thanks for you help

@echasnovski
Copy link
Owner Author

Could you please indicate how I can do this as you mentioned with: "MiniIcons.get()".

I do not really understand the actual problem here.

Icons (either in MiniIcons.get() or in its 'nvim-web-devicons' mock) are always returned without extra spaces. This way plugins (like file explorer, statusline, etc) can decide themselves how much padding they want when using the icon.

Most plugins which use icons already add a single space to the right of the icon. This is usually to allow those icons to be shown wider than a single cell, if both terminal emulator and font support this. I believe this is the case in the earlier screenshots.

In theory, you can monkey-patch MiniIcons.get() by writing a wrapper that appends a single space to the icon. I personally don't think this is a good idea, so I will refrain from giving an actual code for this (although it should be pretty compact).

@folke
Copy link
Contributor

folke commented Jul 21, 2024

I don't know the exact settings, but most terminals can be configured on how to show those icons.
It also depends on the nerd font you use.
This is how those icons look like in wezterm:
image

@ibhagwan
Copy link

Hi @echasnovski,

I'm currently working on adding mini.icons first-class support for fzf-lua (almost complete btw), code can be found in the mini_icons branch, most of the logic is in the devicons.lua file.

Some background, in order to achieve lag free-experience, fzf-lua runs heavy duty processing in an external process, AKA fzf-lua's multiprocess which is essentially a neovim lua wrapper to a shell command, responsible for adding git icons, file icons, ctags parsing, etc.

Fzf-lua therefore does not call the get method directly (of either devicons or mini.icons), but instead maintains it's own set of lookup tables which are then retrieved over RPC from the external process.

This works great with nvim-web-devicons as all icons/files are defined in the plugin code which isn't the case with mini.icons, as I'm sure I don't need to explain this to you I'll add this bit for other readers, when an extension isn't found in mini's original lookup tables a call will be made to vim.filetype.match which will then attempt to detect the filetype based on many different factors (filename, extension, buffer contents, etc), if found, the extension will be cached by mini to prevent additonal calls to vim.filetype.match.

The above presents an issue for fzf-lua, while I can easily replicate this logic, due to the get_icon call happening inside a libuv process loop, the call to vim.filetype.match fails with:

Error executing luv callback:
vim/_options.lua:150: E5560: Vimscript function must not be called in a lua loop callback

The above isn't easy to work around from the external process with vim.schedule and I'm not sure if there's another workaround aside from replicating the vim.filetype.match code inside fzf-lua.

So, for now, I just added a simple check if the extension is the same as the filetype which is obviously imprefect and fails at the simple case of file.yml as the filetype for YAML files is yaml (you can also see the failure for detecting Makefile as it relies on the vim.filetype,match fallback):
image

Said code:
https://github.com/ibhagwan/fzf-lua/blob/8af1836c94ca8668f5175be9e44a9cc0bf1de8f3/lua/fzf-lua/devicons.lua#L411-L436

With fzf-lua vim.in_fast_event() is always true when this is called

  -- mini.icons supports lookup by filetype
  if not icon and STATE.icons.by_filetype then
    if not vim.in_fast_event() then
      local ft = vim.filetype.match({ filename = filename })
      if ft and #ft > 0 then
        local by_ft = STATE.icons.by_filetype[ft]
        if by_ft then
          icon, color = by_ft.icon, by_ft.color
          -- Store in the corresponding lookup table so we don't
          -- have to call `vim.filetype.match` a second time
          if ext then
            STATE.icons.by_ext[ext] = by_ft
          else
            STATE.icons.by_filename[filename] = by_ft
          end
        end
      end
    elseif ext then
      -- HACK: since we cannot use `vim.filetype.match` we attempt resolving
      -- using extension as filetype, while not accurate it works in many cases
      local by_ft = STATE.icons.by_filetype[ext]
      if by_ft then
        icon, color = by_ft.icon, by_ft.color
      end
    end
  end

If this isn't the right place for this disucssion let me know and I'll create a separate issue or we can also continue on the existing fzf-lua issue: ibhagwan/fzf-lua#1319.

@echasnovski
Copy link
Owner Author

Hi, @ibhagwan!

This all indeed looks fairly complicated :(

... a call will be made to vim.filetype.match which will then attempt to detect the filetype based on many different factors (filename, extension, buffer contents, etc), if found, the extension will be cached by mini to prevent additonal calls to vim.filetype.match.

Apart from small nitpick details (like buffer contents is essentially not used and how cache works, which is subject to change), this is indeed what is done. I am afraid that heavy usage of vim.filetype.match() is a cornerstone of 'mini.icons'.

As it might be a lengthy conversation, let's indeed continue this in ibhagwan/fzf-lua#1319.


In an ideal world, I would have loved to not track special icons for extensions and filenames at all while completely relying on vim.filetype.match(). However, there are several issues with this:

  • Performance: vim.filetype.match() is slow-ish (~0.1ms) which matters when called many times synchronously. The matter is less pressing on Nightly 0.11, but still far from being on 'nvim-web-devicons' level (~0.001ms) which uses only table lookups.
  • Convenience: in order to customize some file names there would need to be a vim.filetype.add() call returning some custom filetype. However, this will affect filetype detection when opening the file.

The current approach of "try extension before vim.filetype.match()" has its sever downsides (for example, it blocks custom icons based on the custom filetype detection which involves known extension; even more so if detection relies on extension and parent directory name like in #1017), so I am currently trying to rework this.

@echasnovski echasnovski pinned this issue Jul 26, 2024
@echasnovski
Copy link
Owner Author

echasnovski commented Jul 26, 2024

After a lot of consideration and profiling on real world cases, I've just pushed several important updates. Most of them are about how "file" category is resolved. Here is a TL;DR:

  • Full path support now not only for user convenience but also has a meaningful outcome: it will be used in vim.filetype.match() as is (not only basename, as was before). This allows more targeted icons based on complex filetype detection. See 6b5125a commit message for more details and performance implications.

    'mini.files' now is also updated to use full path instead of only basename. Maybe the same will be done for 'mini.tabline'.

  • Extension detection now is done directly via MiniIcons.get('extension', ext): if it is a recognized extension (i.e. not fallback), it is used. See fb0faa8 commit message for more details and performance implications.

  • Users can now have full control over which extensions will be even considered during file resolution. This can be done through config.use_file_extension function predicate. Apart from being a nice and concise design for something that is "nice to have", this is needed to counter previous bullet point for filetype detection rules which contradict "extension first" detection. See fb2cbeb commit message and this example usage.

See this comment for demonstration of main benefit of these changes.

To counter increased memory usage, caching now has less duplication and seems to be as optimal as is reasonable. @mehalter, your idea about using less memory in caching was a good catalyst for this improvement.


Profiling "before" and "after" on several real world use cases seems to indicate that in "clean" state there is mostly a slight improvement in both speed and memory usage. But assuming another Lua object containing used full paths (like is done in 'mini.files'), there is a consistent and significant (like 3-4 times) improvement in memory. The only thing worse is a first-call speed for paths without recognizable extensions (especially for 'file.with.many.dots.as.each.will.call.vim.filetype.match' cases).


All in all, I hope that this concludes the overall redesign updates. There are possibly two more functions to consider for exporting (mock_lspkind() from #1018 and set() for targeted setting icon data), after which 'mini.icons' should be at stage of only adding new icon data.

@ibhagwan
Copy link

ibhagwan commented Jul 28, 2024

I have a couple of minor nitpicks :-)

(1) I feel the special icon for README, README.md and README.txt is too big when used in with nerd-fonts that weren't patched as "mono" (requires --use-single-width-glyphs).

I personally patch my fonts non-monospaced as I feel it looks nicer, the icons are bigger and more distinct, however that causes the specialized README icon to look too big and portrude into the next char:

Non-monospaced glyphs
image

Monospaced glyphs
image

(2) Another issue I have is also realted to monospaced vs non-monospaced, some of the icons (e.g. yaml) are capable of full width/height in a monospaced font and some aren't, compare the size of the yaml icon between the two screenshots below:

Non-monospaced glyphs (you can also see here how the README.md icon "sticks out" compared to the others)
image

Monospaced glyphs (note the unproportional height of the yaml icon)
image

@AlexKurisu
Copy link
Contributor

I have a question: how do i override builtin icon for init.lua? Before,

file = {
	["init.lua"] = { glyph = "󰢱", hl = "MiniIconsAzure" },
},

was enough, now MiniIcons.get("file", "~/.config/nvim/init.lua") still returns old icon even if i override it.

@echasnovski
Copy link
Owner Author

echasnovski commented Jul 28, 2024

All in all, I hope that this concludes the overall redesign updates.

All hope was lost hours after this was posted when #1086 issue surfaced after the latest redesign. Turns out, the "extension" category did not have proper resolution of its own: "my.lua" extension worked (because vim.filetype.match() recognizes that as 'lua' filetype), but "my.mp4" or "my.spec.ts" (with "spec.ts" configured by the user in setup()) did not. Now they do.

Also with "extension" having its own resolution use_file_extension(ext, file) is called only once for the biggest file name suffix after the dot. So using something like ext:sub(-3) ~= 'scm' or vim.endswith(ext, 'scm') is preferable over direct comparison or table lookup.


I have a question: how do i override builtin icon for init.lua?

@AlexKurisu, my bad, didn't have test cases for overriding of built-in file names (only for customizing existing ones). This now should also be resolved on latest main.


I have a couple of minor nitpicks :-)

@ibhagwan, thanks for the nitpicky feedback! Answering it is the least I can do for your troubles of adding first class 'mini.icons' support :)

(1) I feel the special icon for README, README.md and README.txt is too big

I am afraid, this is intentional. See this comment.

(2) Another issue I have is also realted to monospaced vs non-monospaced, some of the icons (e.g. yaml) are capable of full width/height in a monospaced font and some aren't,

We can talk about this on a case by case basis. If I understand the issue, at least the following glyphs are troublesome (which are tall and narrow):

  • yaml filetype with its exclamation point. As it is a dedicated "nf-seti-yml" class icon, it will probably stay.
  • All shell related filetypes/extensions (like 'hello.sh') with its dollar sign. I spent too much time deciding on this one. But as there is no icon with appropriate class dedicated to terminal emulators (I'd like to limit to only using "nf-md-", "nf-seti-", and "nf-custom-*" classes), I decided to go with "nf-seti-shell". Close second was "nf-md-code_greater_than_or_equal". I am open to changing that with more appropriate one, but I couldn't find it after quite literally looking through all icons of aforementioned classes.
  • 'LICENSE' with its keys. Again, it is a dedicated "nf-seti-license". There is also "nf-md-license", but the first one is more descriptive.

If there are some other ones, I am happy to discuss.

Thankfully, all these icons should be easily configurable, so I hope they are not a huge issue.

@gpwns3717
Copy link

How do I differentiate between two completely different filetypes that use the same extension, for example Verilog and V (which are both .v)?

@echasnovski
Copy link
Owner Author

How do I differentiate between two completely different filetypes that use the same extension, for example Verilog and V (which are both .v)?

The best answer is "the support here is limited". Which I think is fairly reasonable, because icons are returned only based on the string (extension name or file path). There is no other data available.

The only thing that can be used here is an elaborate filetype matching based on pattern. Here is an example demonstrating the concept using built-in 'queries/.*%.scm' pattern to match 'query' filetype instead of 'scheme' which matches 'scm' extension. The #1017 discussion has another example with 'yaml.ansible'.

Note: the config.use_file_extension is present specifically to ensure that some extensions are ignored and will be resolved only with vim.filetype.match().

@ibhagwan
Copy link

Thankfully, all these icons should be easily configurable, so I hope they are not a huge issue.

None of this is a big deal, I’ll just create overrides, ty @echasnovski.

@echasnovski
Copy link
Owner Author

After the latest addition of tweak_lsp_kind() (which updates CompletionItemKind and SymbolKind in vim.lsp.protocol in place which can later be used by completion or other LSP-related plugins), I currently think that the overall design work for 'mini.icons' is done. I've decided to postpone implementation of targeted set() until there is an outside need for it.

There will still be a plenty of beta-testing time in case anyone has a new feedback, which I am happy to discuss.

@231tr0n
Copy link

231tr0n commented Jul 30, 2024

After the latest addition of tweak_lsp_kind() (which updates CompletionItemKind and SymbolKind in vim.lsp.protocol in place which can later be used by completion or other LSP-related plugins), I currently think that the overall design work for 'mini.icons' is done. I've decided to postpone implementation of targeted set() until there is an outside need for it.

There will still be a plenty of beta-testing time in case anyone has a new feedback, which I am happy to discuss.

Hi and thank you for this module again. I just had a small doubt with the tweak_lsp_kind() method.
How do I use this with nvim-cmp. Like is there a way to use it. I dont see the pictograms at all after calling the tweak_lsp_kind() method. I read that there was no support of nvim-cmp but can you suggest a hacky way with which we can make this work?

@echasnovski
Copy link
Owner Author

Hi and thank you for this module again. I just had a small doubt with the tweak_lsp_kind() method. How do I use this with nvim-cmp. Like is there a way to use it. I dont see the pictograms at all after calling the tweak_lsp_kind() method. I read that there was no support of nvim-cmp but can you suggest a hacky way with which we can make this work?

You don't have to use it to have icons in 'nvim-cmp'. As far as I understand, implementing proper formatting.format field should be enough. Here is an example:

require('nvim-cmp').setup({
-- ...
  formatting = {
    format = function(_, vim_item)
      local icon, hl = MiniIcons.get("lsp", vim_item.kind)
      vim_item.kind = icon .. " " .. vim_item.kind
      vim_item.kind_hl_group = hl
      return vim_item
    end,
  },
})

@231tr0n
Copy link

231tr0n commented Jul 30, 2024

Hi and thank you for this module again. I just had a small doubt with the tweak_lsp_kind() method. How do I use this with nvim-cmp. Like is there a way to use it. I dont see the pictograms at all after calling the tweak_lsp_kind() method. I read that there was no support of nvim-cmp but can you suggest a hacky way with which we can make this work?

You don't have to use it to have icons in 'nvim-cmp'. As far as I understand, implementing proper formatting.format field should be enough. Here is an example:

require('nvim-cmp').setup({
-- ...
  formatting = {
    format = function(_, vim_item)
      local icon, hl = MiniIcons.get("lsp", vim_item.kind)
      vim_item.kind = icon .. " " .. vim_item.kind
      vim_item.kind_hl_group = hl
      return vim_item
    end,
  },
})

Works like a charm. Thank you soo much.

@bassamsdata
Copy link

An enhanced version of nvim-cmp code, as we have other plugins that utilize cmp (especially AI plugins). I prefer icons only, so I commented out the names of the sources.

-- somewhere outside the setup function:
local kind_icons = {
  ellipsis_char = "...",
  Copilot = "",
  Supermaven = "",
  Codeium = "",
  otter = "🦦",
  Cody = "",
  cmp_r = "R",
}

-- in the setup function
formatting = {
  format = function(_, vim_item)
    local icon, hl, is_default =
      require("mini.icons").get("lsp", vim_item.kind)
    -- If the icon is not found in mini.icons (is_default is true), use the fallback
    if is_default then
      icon = kind_icons[vim_item.kind] or "󰞋"
      hl = "CmpItemKind" .. vim_item.kind
    end
    vim_item.kind = icon -- .. " " .. vim_item.kind
    vim_item.kind_hl_group = hl
    return vim_item
  end,
  fields = { "kind", "abbr" }, -- "menu",
},

@wenjinnn
Copy link

An enhanced version of nvim-cmp code, as we have other plugins that utilize cmp (especially AI plugins). I prefer icons only, so I commented out the names of the sources.

-- somewhere outside the setup function:
local kind_icons = {
  ellipsis_char = "...",
  Copilot = "",
  Supermaven = "",
  Codeium = "",
  otter = "🦦",
  Cody = "",
  cmp_r = "R",
}

-- in the setup function
formatting = {
  format = function(_, vim_item)
    local icon, hl, is_default =
      require("mini.icons").get("lsp", vim_item.kind)
    -- If the icon is not found in mini.icons (is_default is true), use the fallback
    if is_default then
      icon = kind_icons[vim_item.kind] or "󰞋"
      hl = "CmpItemKind" .. vim_item.kind
    end
    vim_item.kind = icon -- .. " " .. vim_item.kind
    vim_item.kind_hl_group = hl
    return vim_item
  end,
  fields = { "kind", "abbr" }, -- "menu",
},

I must say that be careful to remove menu field from formatting, cause in some LSP like jdtls (not sure if other LSP has the same behavior), menu source are not provide source information but method signature or something useful message.

image

The 2th column is the menu field from jdtls.

@echasnovski
Copy link
Owner Author

An enhanced version of nvim-cmp code, as we have other plugins that utilize cmp (especially AI plugins). I prefer icons only, so I commented out the names of the sources.

@bassamsdata, I would still suggest using the version of code example from my comment, but with adding custom icon data in require('mini.icons').setup() (be sure to use lowercase, as documented). For example:

require('mini.icons').setup({
  lsp = {
    ellipsis_char = { glyph = '', hl = 'MiniIconsRed' },
    copilot = { glyph = '', hl = 'MiniIconsOrange' },
    supermaven = { glyph = '', hl = 'MiniIconsYellow' },
    codeium = { glyph = '', hl = 'MiniIconsGreen' },
    otter = { glyph = '🦦', hl = 'MiniIconsCyan' },
    cody = { glyph = '', hl = 'MiniIconsAzure' },
    cmp_r = { glyph = 'R', hl = 'MiniIconsBlue' },
  },
})

@bassamsdata
Copy link

I would still suggest using the version of code example from my comment, but with adding custom icon data in require('mini.icons').setup()

Thank you. True, this approach has the advantage of adding highlights and using the icons elsewhere while reducing the amount of necessary code.

@echasnovski
Copy link
Owner Author

Not exactly an update on 'mini.icons', but closely related.

'mini.files' now computes prefix (which by default uses 'mini.icons' if it is enabled) only for visible items in preview. This was done to reduce the delay when first time previewing directory with lots of entries (as MiniIcons.get() has not yet cached outputs). So navigating up and down with preview and 'mini.icons' enabled should now be smoother.

First focusing on path still may result in a delay though :( At least until 'mini.nvim' drops support for Neovim 0.9.5 which doesn't have inline extmarks.

lervag added a commit to lervag/dotnvim that referenced this issue Sep 21, 2024
@echasnovski
Copy link
Owner Author

Thanks everyone for the amazing feedback!

With the release of 0.14.0, 'mini.icons' is now out of beta-testing!

@echasnovski echasnovski unpinned this issue Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests