Bug: Files list renders with ~1s delay when `mini.nvim` is installed and a file with unknown extension exists

[somnam]

RTFM Checklist

• [X] I have searched exisiting issues / discussions
• [ ] I have read the Wiki including the Advanced section
• [ ] I have read man fzf / I am well versed in shell fzf

Operating system

Mac

Shell

bash

Neovim version (nvim --version)

NVIM v0.10.1

Fzf version (fzf --version)

0.52.1 (6432f00)

Output of :lua print(os.getenv('FZF_DEFAULT_OPTS'))

nil

Is the problem reproducible with mini.sh?

• [ ] My issue is reproducible with mini.sh
• [ ] My issue IS NOT reproducible with mini.sh
• [X] I have not tested with mini.sh (not relevant, requires LSP, Windows, etc)

Fzf-lua configuration

-- No specific setup options are required but `mini.nvim` needs to be installed.
require('fzf-lua').setup({})

Describe the bug / steps to reproduce

The latest version of fzf-lua enables icon support when mini.nvim plugin is installed. When using files list (:FzfLua files), the list renders with ~1s delay when mini.nvim is installed and a file with unknown extension exists in current path.

The render delay happens due to overriding vim.fn.fnamemodify with a custom lua implementation in module lua/fzf-lua/path.lua and function ft_match:

  vim.fn.fnamemodify = M._fnamemodify

The lua override is called for files of unknown type and each call adds a noticeable lag. Triggering this issue can be done with a simple lua script:

1. Create an empty file file.bak in current path.
2. Create a script file.lua in current path with following contents:

   
   _fnamemodify = function(fname, mods)
   return fname
   end

vim.fn.fnamemodify = _fnamemodify

local _, res = pcall(vim.filetype.match, { filename = vim.fn.getcwd() .. "/file.bak" }) print(res)

3. Run the script:
```shell
➜ time nvim --headless --noplugin -l file.lua
nil

real    0m0.636s
user    0m0.622s
sys     0m0.009s

It seems that a more nested path give a longer delay for some reason.

When debugging this problem I've temporarily commented out both vim.env and vim.fn.fnamemodify overrides in lua/fzf-lua/path.lua and didn't notice the error stated in the file (E5560: Vimscript function must not be called in a lua loop callback) afterwards when using the plugin. Could this error exist only in some versions of neovim? Maybe the override could be enabled only if version is lower than some specific release? In 0.10.1 it tentatively seems to be not needed.

[ibhagwan]

The render delay happens due to overriding vim.fn.fnamemodify

Are you sure that this is what’s causing the delay? The override implements only :t for file tail (basename):

https://github.com/ibhagwan/fzf-lua/blob/cc8b1ee2868e10fb4f4c00ca827a5b2af7a9b88a/lua/fzf-lua/path.lua#L575-L580

The delay is most likely due to vim.filetype.match which is a feature of using mini.icons and there’s not much I can do about it, while I cache extensions and file names for further lookups there will still be paths which will end up calling vim.filetype.match (the fallback) which is IMHO in the crux of the issue.

Unless mini.icons implements more static extensions/file name and a logic that doesn’t rely on calling vim.filetype.match there’s nothing else I can do about this, for further reading see @echasnovski’s input in https://github.com/echasnovski/mini.nvim/issues/1007#issuecomment-2242499529 as well as the discussions in #1319.

[ibhagwan]

About this:

When debugging this problem I've temporarily commented out both vim.env and vim.fn.fnamemodify overrides in lua/fzf-lua/path.lua and didn't notice the error stated in the file (E5560: Vimscript function must not be called in a lua loop callback) afterwards when using the plugin.

The call is wrapped in pcall so it won’t generate the error but it would also not display an icon for all paths where the vim.filetype.match is required (a lot):

https://github.com/ibhagwan/fzf-lua/blob/cc8b1ee2868e10fb4f4c00ca827a5b2af7a9b88a/lua/fzf-lua/path.lua#L603-L607

[somnam]

Are you sure that this is what’s causing the delay?

Please try out the attached lua script with an empty file of unknown type. When vim.fn.fnamemodify is overwritten with a custom lua function calls to vim.filetype.match seem to last a long time:

_fnamemodify = function(fname, mods)
  return fname
end

vim.fn.fnamemodify = _fnamemodify

The override can be an echo function like in the example, not even implementing :t but returning instantly. Running it with nvim having disabled plugins shows the time delay.

[ibhagwan]

Please try out the attached lua script with an empty file of unknown type.

Ty for the debugging efforts @somnam, I will try.

However, as I mentioned above, without this override we won’t have the vim.filetype.match severely impacting mini.icons functionality.

[somnam]

Oh, I get it. Other file types don't display correctly without the override. That's indeed limiting. Thought it's strange that the lua override generates such a delay. Do you have an idea what might cause it?

[ibhagwan]

Oh, I get it. Other file types don't display correctly without the override. That's indeed limiting. Thought it's strange that the lua override generates such a delay. Do you have an idea what might cause it?

Perhaps this:

The override can be an echo function like in the example, not even implementing :t but returning instantly. Running it with nvim having disabled plugins shows the time delay.

The above is not a good test, not doing what the modifier is requesting returns full paths (instead of, for example, just the basename when :t is requested), that, in fact causes the runtime code to take different paths inside vim.filetype.match which falls back into string pattern matching, if it needs to exhaust a lot of patterns, it will generate a delay.

From my experiments only :t and :p (which I didn’t implement) are called in our case, that means, either :p needs to be implemented to make sure the same code path is taken or I missed something and additional modifiers need to be implemented.

[somnam]

The above is not a good test, not doing what the modifier is requesting returns full paths

Yeah, I'm not sure what's the issue here. When I use the _fnamemodify fallback from lua/fzf-lua/path.lua that implements :t in my script instead of my echo override I get the same long delay. Uhm, maybe it's due to overriding the vim builtin with a lua function and that will always cause a delay. I'm not familiar with neovim internals tho.

Thanks for looking into this, probably disabling the icons globally is the quickest fix.

[ibhagwan]

The above is not a good test, not doing what the modifier is requesting returns full paths

Yeah, I'm not sure what's the issue here. When I use the _fnamemodify fallback from lua/fzf-lua/path.lua that implements :t in my script instead of my echo override I get the same long delay. Uhm, maybe it's due to overriding the vim builtin with a lua function and that will always cause a delay. I'm not familiar with neovim internals tho.

I just did some testing, implementing both :p and :t, if anything it made things worse (as probably more patterns needed to be tested against).

Thanks for looking into this, probably disabling the icons globally is the quickest fix.

ATM, there is no applicable workaround to this issue, this is probably the most reasonable medium, to not use mini.icons in the auto-detect algo, if anyone wants to use mini they can always add defaults = { file_icons = 'mini' } to their setup call.

Unfortunately having to rely on vim.filetype.match isn’t a good fit for anything that requires performance, although I’m pretty sure the second+ run(s) will always be faster (due to caching) the first time lag/delay is annoying and there will still be cases where cache isn’t applicable (I.e. persistent delay).

[ibhagwan]

https://github.com/ibhagwan/fzf-lua/commit/769b6636af07ea4587e6c06067d8fe9fb0629390

@somnam, post the above commit only nvim-web-devicons is auto-detected and enabled, mini.icons will require an additonal manual step:

require("fzf-lua").setup({ defaults = { file_icons = "mini" } })

[echasnovski]

@somnam, post the above commit only nvim-web-devicons is auto-detected and enabled, mini.icons will require an additonal manual step:

@ibhagwan, this is really unfortunate. At least for these reasons:

• The "... there will still be cases where cache isn’t applicable (I.e. persistent delay)" is not the case with upstream MiniIcons.get(). All outputs are persistently cached and the second+ call with the same arguments is always faster.
• The git_icons is enabled by default yet Git is notoriously slow and is for me the persistent cause of significant delay on large projects (until you told me about an explicit opt-out git_icons = false, which is indeed faster).
• On Nightly 0.11 vim.filetype.match() was optimized for quite visible performance gain (about 4x faster). So at least auto-enabling there might be an option.

---

One thing I noticed is that there is indeed not that much difference between the first and second execution of :FzfLua files git_icons=false. For reference, I tend to use this repo for profiling when I need "big realistic repo" (like for 'mini.pick' and 'mini.icons'), fzf finds 1010325 files there. Here is data from my profiling (measured manually with physical timer by hand; Nightly version is NVIM v0.11.0-dev-504+gaa853f362):

file_icons	Execution number	Neovim version	Duration (seconds)
'mini'	first	0.10.0	11.6
'mini'	second	0.10.0	10.3
'mini'	first	Nightly	6.36
'mini'	second	Nightly	5.5
'devicons'	first	0.10.0	2.75
'devicons'	second	0.10.0	2.75
'devicons'	first	Nightly	2.72
'devicons'	second	Nightly	2.75
false	first	0.10.0	1.0
false	first	Nightly	0.91

The second execution in session of 'mini' case should be much faster in theory.

---

A bit of exploration showed that require('fzf-lua.devicons').PLUGIN._state.icons indeed doesn't cache all cases, but only extensions and filenames returned by MiniIcons.list('file'). Using MiniIcons.get() directly would have resulted into all 1010325 cases being cached. However I think this might be bad for 'fzf-lua' as it needs to communicate that cache through RPC each time, right?

But the general idea of "vim.filetype.match() should not be called on second+ call for same input" seems achievable. For example, caching file name when it does not have extension should improve things (as there are 41239 such files which on my machine with original vim.filetype.match() takes about 8 seconds). So I applied the following patch:

Patch with caching filenames without extension

```diff diff --git a/lua/fzf-lua/devicons.lua b/lua/fzf-lua/devicons.lua index c1ba2be..1e35d6b 100644 --- a/lua/fzf-lua/devicons.lua +++ b/lua/fzf-lua/devicons.lua @@ -470,6 +470,11 @@ M.get_devicon = function(filepath, extensionOverride) icon = icon or STATE.default_icon.icon color = color or STATE.default_icon.color + if not ext then + local name = STATE.icons.by_filename_case_sensitive and filename or filename:lower() + STATE.icons.by_filename[name] = { icon = icon, color = color } + end + if STATE.icon_padding then icon = icon .. STATE.icon_padding end ```

After that, the same profiling for :FzfLua files git_icons=false gave this:

file_icons	Execution number	Neovim version	Duration (seconds)
'mini'	first	0.10.0	10.75
'mini'	second	0.10.0	5.38
'mini'	first	Nightly	6.47
'mini'	second	Nightly	5.5

Massive improvement for second execution on 0.10.0. But, as there is no improvement on Nightly, my instinct says that it is not vim.filetype.match() calls that causes the difference extra ~2.75 seconds compared to file_icons = 'devicons'.

---

Sorry for the wall of text. Was curious about the nature of delay. Hope it all helps :)

[echasnovski]

@somnam, having the whole second as delay supposedly due to vim.filetype.match() seems quite big for me. Could you clarify, how this delay manifests itself? Ideally, make a screencast.

Also, assuming you have 'mini.misc' available, could you, please execute the following commands and post what it added to the following buffer:

:lua require('mini.misc').setup({ make_global = { 'put', 'put_text', 'stat_summary', 'bench_time' } })
:lua buf_id = vim.api.nvim_create_buf(false, true)
:lua put_text(stat_summary(bench_time(vim.filetype.match, 1000, { filename = 'should-not-match', buf = buf_id })))

For me it gives something like this:

{
  maximum = 0.000738008,
  mean = 0.000245835569,
  median = 0.00024022,
  minimum = 0.00023772,
  n = 1000,
  sd = 2.7421614662341e-05
}

[ibhagwan]

@echasnovski, tysm for the detailed information and testing!

The "... there will still be cases where cache isn’t applicable (I.e. persistent delay)" is not the case with upstream MiniIcons.get(). All outputs are persistently cached and the second+ call with the same arguments is always faster.

Correct, fzf-lua doesn’t use the upstream MiniIcons.get and does not cache file names with no extension and full paths, this would require much more memory and is bad for many different reasons.

The git_icons is enabled by default yet Git is notoriously slow and is for me the persistent cause of significant delay on large projects (until you told me about an explicit opt-out git_icons = false, which is indeed faster).

That is 100% true, the only reason this remains true by default is because I dislike breaking changes and git icons defaulted to true for a long time until I realized it was causing delay in large repos (granted it has to be pretty big), I also added a special warning so users are aware if it’s exceptionally slow - doing so with mini would require profiling each icon get call, not very practical.

There might be a case to default git_icons to false even though it would be considered “breaking” (btw, it’s false in many of the offered :FzfLua profiles).

On Nightly 0.11 vim.filetype.match() was optimized for quite visible performance gain (about 4x faster). So at least auto-enabling there might be an option.

If nightly is much better we can revisit this but it’s not the only reason it makes sense to default to false, given the extreme popularity of mini.nvim I expect 99% of the users to have mini installed which means loading mini.icons automatically will succeed, even for those who do not wish to use icons, this differs from users who wish to use icons in their config and therefore install an icons package specifically.

A bit of exploration showed that require('fzf-lua.devicons').PLUGIN._state.icons indeed doesn't cache all cases, but only extensions and filenames returned by MiniIcons.list('file'). Using MiniIcons.get() directly would have resulted into all 1010325 cases being cached. However I think this might be bad for 'fzf-lua' as it needs to communicate that cache through RPC each time, right?

Correct, as I mentioned in my first bullet point caching all paths similar to mini is impractical for fzf-lua, the memory requirement could also result in OOM in big repos and other issues resulting from an exceptionally big cache.

But the general idea of "vim.filetype.match() should not be called on second+ call for same input" seems achievable. For example, caching file name when it does not have extension should improve things (as there are 41239 such files which on my machine with original vim.filetype.match() takes about 8 seconds). So I applied the following patch:

While it might work for your machine could there be other (extreme cases) where it would result in worse performance (many random files which would require an even larger cache)?

Also, this is where it starts to become questionable, if you cached only the file name you’ll end up potentially a wrong icon sometimes, filetype is determined by the full path (using full path pattern matching by the runtime), but if you cached by file name only you will have set the icon to the one retuned by the first call.

NOTE: this is in fact what currently happens with some extensions, since we cache the extension only and not the full path, the first seen extension would be the one for all future occurrences of the same extension regardless of the path, this is a bug by design as I wanted to minimizes the cache size.

Massive improvement for second execution on 0.10.0.

That is indeed a big improvement on nightly.

Sorry for the wall of text. Was curious about the nature of delay. Hope it all helps :)

Always does, ty @echasnovski!

[echasnovski]

If nightly is much better we can revisit this but it’s not the only reason it makes sense to default to false, given the extreme popularity of mini.nvim I expect 99% of the users to have mini installed which means loading mini.icons automatically will succeed, even for those who do not wish to use icons, this differs from users who wish to use icons in their config and therefore install an icons package specifically.

Hence the official suggestion for plugin authors to use icons only if _G.MiniIcons variable is present. This approach is used for any 'mini.nvim' module to check whether the user called its setup(). So this becomes:

• If _G.MiniIcons is not nil -> user called require('mini.icons').setup() -> use icons.
• If _G.MiniIcons is nil -> user did not call require('mini.icons').setup() -> don't use icons.

While it might work for your machine could there be other (extreme cases) where it would result in worse performance (many random files which would require an even larger cache)?

Sure, with random files which don't contain dot this might happen to a certain extent. However, a reference project is built from real world repos and it contains about 4.1% files without extension (41239 out of 1010427). From which only about 1.9% (19102) are unique and will be used in cache. I think caching 1.9% of searched files will be acceptable for most users. Especially if they use 'mini.icons' which already has bigger cache.

Also, this is where it starts to become questionable, if you cached only the file name you’ll end up potentially a wrong icon sometimes, filetype is determined by the full path (using full path pattern matching by the runtime), but if you cached by file name only you will have set the icon to the one retuned by the first call.

As far as I understand, filetype matching is currently done based on basename, hence the caching suggestion. Yes, matching filetype based on full path was added yesterday and it indeed results in a fuller matching.

In vast majority of real world cases (I feel like over 99.9%) relying on only extension and basename is enough. The other cases are just cherry on top. So I think this should be a good compromise in 'fzf-lua'.

Note: by the way, filetype matching is better when a dummy buffer is supplied (the reason is roughly that some file names are intentionally not matched because the answer should differ on buffer content). Like here. This should also resolve this part.

[ibhagwan]

Hence the official suggestion for plugin authors to use icons only if _G.MiniIcons variable is present.

That’s a good idea, I know about this I just didn’t think about it, the code already checks the variable exists to make sure setup was called :)

https://github.com/ibhagwan/fzf-lua/blob/769b6636af07ea4587e6c06067d8fe9fb0629390/lua/fzf-lua/devicons.lua#L183-L191

Btw, I forgot to ask you if a refresh the icon colors is ever needed? with nvim-web-devicons I reload the icons once bg changes from dark to light, are the MiniIcons.. hl groups ever updated?

As far as I understand, filetype matching is currently done based on basename, hence the caching suggestion. Yes, matching filetype based on full path was added yesterday and it indeed results in a fuller matching.

Correct, that’s because I wrote this code before your recent changes, I briefly tested full path matching and it does impact performance a bit so I wanted to test it more before (if?) I make the change to match mini’s internal get.

In vast majority of real world cases (I feel like over 99.9%) relying on only extension and basename is enough. The other cases are just cherry on top. So I think this should be a good compromise in 'fzf-lua'.

That was indeed my logic.

Note: by the way, filetype matching is better when a dummy buffer is supplied (the reason is roughly that some file names are intentionally not matched because the answer should differ on buffer content). Like here. This should also resolve this part.

I saw that and wanted to avoid the scratch buffer bit as it does go into a code path internally (in the runtime) that I’d like to avoid as it could impact performance or potentially break the monkey patch I wrote( for E5560), the txt file issue I would consider an upstream bug, I don’t see any reason why an empty buffer in the call args should change the result of a match, especially with a case as simple as txt.

[ibhagwan]

@echasnovski https://github.com/ibhagwan/fzf-lua/commit/82806dced4931ca768fbdad6b6d26a695ad03d04

• Auto enables mini.icons if _G.MiniIcons is present
• Cache filenames so vim.filetype.match will not be called twice for filenames without extension (we can see how that goes if users are having issues with memory consumption)

Performace comparison of libuv line processing (i.e. adding icons):

:FzfLua files file_icons=mini profile=true query=[DEBUG fzf_opts.--tiebreak=index

First run

Second run

[ibhagwan]

Btw, for comparison I ran the libuv profiler with mini on the latest nightly, it is indeed much better:

First run, same folder as above

Second run matches 0.10.0 as vim.filetype.match is never called a second time

[somnam]

@echasnovski : It seems that the reason for the observed delay isn't directly vim.filetype.match but the override of vim.fn.fnamemodify with a custom lua function in the code: https://github.com/ibhagwan/fzf-lua/blob/cc8b1ee2868e10fb4f4c00ca827a5b2af7a9b88a/lua/fzf-lua/path.lua#L595-L599

In the initial post a minimal script was added that overrides this method the same way with a simple echo function and a timing of calling vim.filetype.match afterwards on a file with unknown extension (file.bak):

_fnamemodify = function(fname, mods)
  return fname
end

vim.fn.fnamemodify = _fnamemodify

local _, res = pcall(vim.filetype.match, { filename = vim.fn.getcwd() .. "/file.bak" })
print(res)

The result matches with the observed delay in fzf-lua ui:

➜ time nvim --headless --noplugin -l file.lua
nil

real    0m0.636s
user    0m0.622s
sys     0m0.009s

The same delay can be also observed when the override method is same as in fzf-lua code:

M._fnamemodify = function(fname, mods)
  if mods == ":t" then
    return M.tail(fname)
  end
  return fname
end

vim.fn.fnamemodify = M._fnamemodify

local _, res = pcall(vim.filetype.match, { filename = vim.fn.getcwd() .. "/file.bak" })
print(res)

Results in:

➜ time nvim --headless --noplugin -l file.lua
nil

real    0m0.605s
user    0m0.589s
sys     0m0.009s

Removing the custom override reduces the run time of the script by a factor of 20x:

local _, res = pcall(vim.filetype.match, { filename = vim.fn.getcwd() .. "/file.bak" })
print(res)

➜ time nvim --headless --noplugin -l file.lua
nil

real    0m0.031s
user    0m0.014s
sys     0m0.010s

I hope these timings clarify a bit what was meant to be conveyed in the initial post.

The same time improvement was also observed after commenting out the override of vim.fn.fnamemodify in fzf-lua code, but for some reason the icon for the known file type didn't load correctly then:

[ibhagwan]

but for some reason the icon for the known file type didn't load correctly then:

I already explained the reason in https://github.com/ibhagwan/fzf-lua/issues/1358#issuecomment-2253418968, it’s because filetype match now fails with E5560 (in the pcall).

[echasnovski]

Btw, I forgot to ask you if a refresh the icon colors is ever needed? with nvim-web-devicons I reload the icons once bg changes from dark to light, are the MiniIcons.. hl groups ever updated?

'mini.icons' intentionally is designed only around highlight groups. Built-in icon data use only fixed set of 9 color groups, users can add any highlight groups in config as they like.

The highlight groups in icon data won't change between different color schemes, but as 'fzf-lua' relies on colors then they need to be updated because they might change. If highlighting was done with Neovim's utilities using only highlight groups, then it would have been no need to reload colors.

Correct, that’s because I wrote this code before your recent changes, I briefly tested full path matching and it does impact performance a bit so I wanted to test it more before (if?) I make the change to match mini’s internal get.

As a heads-up, I think there will be one more small-ish change to how "file" and "extension" is resolved (due to surfacing of echasnovski/mini.nvim#1086). I don't think this will affect 'fzf-lua' mocking, but still.

... the txt file issue I would consider an upstream bug, I don’t see any reason why an empty buffer in the call args should change the result of a match, especially with a case as simple as txt.

As it was explained to me, the idea is to mimic Vim's filetype detection to the dot, which makes it easier to maintain. The logic is to rely on content checking to not return 'text' for help files. While I agree that particular case of 'txt' can be improved, there are unfortunately more cases like this. Basically, if filetype detection relies on content, it will not correctly detect filetype. Here are some notable examples where presence of buf makes a difference:

• The 'ts' extension is not recognized as 'typescript' filetype, because it might be detected as 'xml'.
• The 'bash' extension is not recognized as 'sh' filetype, because it might have different flavors.
• And many more...

I initially thought that using contents = { '' } in vim.filetype.match() would be enough, but it is not. Which indeed seems to be unintended, but I do not have energy right now for fixing it upstream.

• Auto enables mini.icons if _G.MiniIcons is present

That's nice. Thank you!

I've another thing to nit pick even here. Users of 'mini.icons' might have both _G.MiniIcons variable and 'nvim-web-devicons' module (due to MiniIcons.mock_nvim_web_devicons() which is present to make it work for plugins which don't yet have support for 'mini.icons'), while users of 'nvim-web-devicons' will likely not have _G.MiniIcons variable. I'd say that preferring _G.MiniIcons test is more versatile, but I hope that eventually it will be not needed for users to have. So not really sure if its worth changing.

Btw, for comparison I ran the libuv profiler with mini on the latest nightly, it is indeed much better:

I sure hope so that spending a week on profiling, refactoring, improving (and arguing that improvement should be merged) was worth it :)

@echasnovski : It seems that the reason for the observed delay isn't directly vim.filetype.match but the override of vim.fn.fnamemodify with a custom lua function in the code: ... The result matches with the observed delay in fzf-lua ui:

Ah, that delay. Yeah, it is visibly less than a second for me, but it is there. With those examples I understand it better, thanks!

It is indeed weird and just too big of a difference, but I think I have a clue of what's going on.

There is no such delay if your examples use almost any different file name which doesn't match anything. For example, replace '/file.bak' with '/no-detection-here'. This tells that 'file.bak' is special. Which indeed it is, as it is a part of "ignored" extensions, which in turn use vim.fn.fnamemodify(file, ':r') to remove last extension. As mocks return same as input in this case, there is seemingly infinite loop here. @ibhagwan, taking ':r' modifier into account seems to solve the issue. This seems like fixing the delay:

diff --git a/lua/fzf-lua/path.lua b/lua/fzf-lua/path.lua
index 885d683..bac4e83 100644
--- a/lua/fzf-lua/path.lua
+++ b/lua/fzf-lua/path.lua
@@ -576,6 +576,9 @@ M._fnamemodify = function(fname, mods)
   if mods == ":t" then
     return M.tail(fname)
   end
+  if mods == ":r" then
+    return (fname:gsub("%.[^.]*$", ""))
+  end
   return fname
 end

Edit: tweaked the patch with correct gsub().

[echasnovski]

I've another thing to nit pick even here. Users of 'mini.icons' might have both _G.MiniIcons variable and 'nvim-web-devicons' module (due to MiniIcons.mock_nvim_web_devicons() which is present to make it work for plugins which don't yet have support for 'mini.icons'), while users of 'nvim-web-devicons' will likely not have _G.MiniIcons variable. I'd say that preferring _G.MiniIcons test is more versatile, but I hope that eventually it will be not needed for users to have. So not really sure if its worth changing.

@ibhagwan, I've just been bitten by it myself :rofl: (as I have both _G.MiniIcons and MiniIcons.mock_nvim_web_devicons()). The symptoms are very fast execution at expense of very few icons shown. This is because since yesterday there are significantly less explicitly supported extensions returned in MiniIcons.list('extensions') because there is no need in them anymore. As it is used inside mocking of require('nvim-web-devicons').get_icons_by_extension(), it makes 'fzf-lua' think that there are significantly less supported extensions then there is.

I am not sure which approach you'd find better here, but I thought leaving this here for better visibility should not be a bad thing.

[ibhagwan]

The highlight groups in icon data won't change between different color schemes, but as 'fzf-lua' relies on colors then they need to be updated because they might change. If highlighting was done with Neovim's utilities using only highlight groups, then it would have been no need to reload colors.

Not possible, the headless neovim wrapper is an external process unaware of the highlight groups in the main thread, these need to be queried from the main thread and relayed as colors.

Since there’s a finite amount of hl groups I guess there’s no harm in resolving these each time (not happening currently).

As a heads-up, I think there will be one more small-ish change to how "file" and "extension" is resolved (due to surfacing of https://github.com/echasnovski/mini.nvim/issues/1086). I don't think this will affect 'fzf-lua' mocking, but still.

fzf-lua uses its own path utilities to extract the extension, I don’t think it would change anything.

Here are some notable examples where presence of buf makes a difference:

I know about these, I was referring to the difference in resolving between having no buffer and a scratch empty buffer (as you have in mini).

I've another thing to nit pick even here. Users of 'mini.icons' might have both _G.MiniIcons variable and 'nvim-web-devicons' module (due to MiniIcons.mock_nvim_web_devicons() which is present to make it work for plugins which don't yet have support for 'mini.icons'), while users of 'nvim-web-devicons' will likely not have _G.MiniIcons variable. I'd say that preferring _G.MiniIcons test is more versatile, but I hope that eventually it will be not needed for users to have. So not really sure if its worth changing.

The first class mini support is much better than the mock_nvim_web_devicons() which will have many icons missing (due to not calling vim.filetype.detect).

So not really sure if its worth changing.

Shouldn’t be a big deal to change, I can check if the package path contains mini so I’ll know it’s the mock.

I sure hope so that spending a week on profiling, refactoring, improving (and arguing that improvement should be merged) was worth it :)

I wasn’t aware this was your handiwork, chapeau :)

taking ':r' modifier into account seems to solve the issue.

Ty for the debugging @echasnovski! I’ll add :r support.

I've just been bitten by it myself 🤣 (as I have both _G.MiniIcons and MiniIcons.mock_nvim_web_devicons())

I was just replying to you about this, easy solution with self._ package:match.

[somnam]

@echasnovski: Thank you for finding the infinite loop case, that fully explains the observed behaviour 🎉 .

[echasnovski]

Here are some notable examples where presence of buf makes a difference:

I know about these, I was referring to the difference in resolving between having no buffer and a scratch empty buffer (as you have in mini).

Ah, yeah, maybe that makes sense.

I've just tried something, but it will also error with E5560 due to vim.api.nvim_buf_get_lines() and vim.api.nvim_buf_line_count(). Mocking them seems to fix the issue. Here is the patch:

More mocking to the god of mocking

```diff diff --git a/lua/fzf-lua/path.lua b/lua/fzf-lua/path.lua index 885d683..44cb859 100644 --- a/lua/fzf-lua/path.lua +++ b/lua/fzf-lua/path.lua @@ -585,6 +585,9 @@ M._env = setmetatable({}, { end }) +M._nvim_buf_get_lines = function() return { "" } end +M._nvim_buf_line_count = function() return 1 end + function M.ft_match(args) if not args or not args.filename then error('At least "filename" needs to be specified') @@ -595,14 +598,20 @@ function M.ft_match(args) -- E5560: Vimscript function must not be called in a lua loop callback local _env = vim.env local _fnamemodify = vim.fn.fnamemodify + local _nvim_buf_get_lines = vim.api.nvim_buf_get_lines + local _nvim_buf_line_count = vim.api.nvim_buf_line_count vim.env = M._env vim.fn.fnamemodify = M._fnamemodify + vim.api.nvim_buf_get_lines = M._nvim_buf_get_lines + vim.api.nvim_buf_line_count = M._nvim_buf_line_count -- Normalize the path and replace "~" to prevent the internal -- `normalize_path` from having to call `vim.env` or `vim.pesc` local fname = M.normalize(M.tilde_to_HOME(args.filename)) - local ok, ft, on_detect = pcall(vim.filetype.match, { filename = fname }) + local ok, ft, on_detect = pcall(vim.filetype.match, { filename = fname, buf = 0 }) vim.fn.fnamemodify = _fnamemodify vim.env = _env + vim.api.nvim_buf_get_lines = _nvim_buf_get_lines + vim.api.nvim_buf_line_count = _nvim_buf_line_count if ok then return ft, on_detect end end ```

This makes it work for 'ts', 'asm' extensions, and others.

There is only one problem I can observe and it is with shell files (like 'sh', 'bash', etc.; the ones which use these detection functions). But those error even in a vim.filetype.match({ filename = 'hello.sh', buf = 0 }) (actual buffer id doesn't matter) inside nvim -u NONE but not with nvim --clean. Which is indeed very strange and looks like a bug.

Accounting for these extensions instead of currently done 'txt' might solve this partially, but not 100%, because there are more detect.bash, detect.sh, etc. cases. Only somehow tweaking the 'fzf-lua' child process (maybe something to do with runtime paths?) or fixing it upstream seems to be the 100% solution. But I am clueless here, I am afraid.

[ibhagwan]

https://github.com/ibhagwan/fzf-lua/commit/d1493cfac8e5c4907759ac827b983fa5aeaf7ad4

@echasnovski @somnam see the latest iteration changes:

• If nvim-web-devicons isn't avilable and the user called mock_nvim_web_devicons fzf-lua will load mini.icons natively, it will also call require("mini.icons") setup if it can't find _G.MiniIcons beacuse why not?

• Added :r modifier with a slight change, reading the docs if the file starts with a dot it should be returned as is: https://github.com/ibhagwan/fzf-lua/blob/d1493cfac8e5c4907759ac827b983fa5aeaf7ad4/lua/fzf-lua/path.lua#L579-L582

• MiniIconsXXX hl groups are now refreshed with each run:

  @echasnovski, if you ever decide to add new fixed hl groups kindly let me know https://github.com/ibhagwan/fzf-lua/blob/d1493cfac8e5c4907759ac827b983fa5aeaf7ad4/lua/fzf-lua/devicons.lua#L178-L198

I've just tried something, but it will also error with E5560 due to vim.api.nvim_buf_get_lines() and vim.api.nvim_buf_line_count(). Mocking them seems to fix the issue. Here is the patch

Ty @echasnovski, I'll test, not a big fan of so many monkey patches but we can give it a shot.

[ibhagwan]

https://github.com/ibhagwan/fzf-lua/commit/6edea7e876477a2a5811c1a172ff6d2a25669e1f

@echasnovski tysm for the improved mock patch, works great!

With this I believe mini.icons first class support is in a great state.

[echasnovski]

• MiniIconsXXX hl groups are now refreshed with each run:

@echasnovski, if you ever decide to add new fixed hl groups kindly let me know

I'll do better: I'll try to not add new highlight groups.

But that said, I think it might have been a good idea to cache actually used in utils.hexcol_from_hl() highlight groups and then use them when color scheme changes. It doesn't need manual tracking, more to the point, and is more robust for users which might want to use their own highlight groups. But using only MiniIconsXxx groups results in cleaner UI, so maybe it is a good thing to discourage using them here :)

With this I believe mini.icons first class support is in a great state.

:rocket:

Just remember about the mentioned earlier '.sh', '.bash', etc. issues if anybody will raise an issue about them not having an icon.

[ibhagwan]

But that said, I think it might have been a good idea to cache actually used in utils.hexcol_from_hl() highlight groups and then use them when color scheme changes. It doesn't need manual tracking, more to the point, and is more robust for users which might want to use their own highlight groups. But using only MiniIconsXxx groups results in cleaner UI, so maybe it is a good thing to discourage using them here :)

Correct, no biggie, I'll make the modifications for that.

Just remember about the mentioned earlier '.sh', '.bash', etc. issues if anybody will raise an issue about them not having an icon.

Already found an issue with sh, will have to see what went wrong with the mock as it fails with autoset_dpi.sh /usr/share/nvim/runtime/lua/vim/filetype/detect.lua:1352: bad argument #1 to 'find' (string expected, got nil)

[echasnovski]

6edea7e

@echasnovski tysm for the improved mock patch, works great!

With this I believe mini.icons first class support is in a great state.

Something seems not right :( On 6edea7e87647 I get inconsistent number of search results with 'mini.icons' enabled. Sometimes even 0. Doesn't seem to be the case on d1493cfac8e5.

Already found an issue with sh, will have to see what went wrong with the mock as it fails with autoset_dpi.sh /usr/share/nvim/runtime/lua/vim/filetype/detect.lua:1352: bad argument #1 to 'find' (string expected, got nil)

It is not necessarily the mocks that are the problem. Native vim.filetype.match() fails on nvim -u NONE but not nvim --clean. See the comment.

[ibhagwan]

https://github.com/ibhagwan/fzf-lua/commit/d0a8c9ac40d12043c1a7fba69820d713c810ec26

@echasnovski, I didn't read this properly the first time, added exceptios for the 4 prolematic types.

[ibhagwan]

https://github.com/ibhagwan/fzf-lua/commit/131d755dbeadb9adb31c54599fa15dd232f92a64

Ok, I think this is the final commit in regard to this issue and mini.icons relating to:

But that said, I think it might have been a good idea to cache actually used in utils.hexcol_from_hl() highlight groups and then use them when color scheme changes. It doesn't need manual tracking, more to the point, and is more robust for users which might want to use their own highlight groups. But using only MiniIconsXxx groups results in cleaner UI, so maybe it is a good thing to discourage using them here :)

Used hl groups are discovered as part of the enumeration of filenames/filetypes/extensions and are resolved to hex colors every time fzf-lua UI is opened.

Default colors

:hi! link MiniIconsAzure ErrorMsg

:hi! clear MiniIconsAzure

[ibhagwan]

@somnam, it seems that with the :r fix the initial enumeration also opens pretty fast:

:FzfLua files file_icons=mini profile=true query=[DEBUG fzf_opts.--tiebreak=index

First open:

Second open:

Compared to the first open times posted in https://github.com/ibhagwan/fzf-lua/issues/1358#issuecomment-2254171146 this is a great improvement, tysm for all the help @echasnovski!