echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.46k stars 172 forks source link

Beta-testing 'mini.icons' #1007

Open echasnovski opened 2 days ago

echasnovski commented 2 days ago

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):

Thanks!

SebasF1349 commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

Work-around to make it work on fzf-lua

  {
    "echasnovski/mini.icons",
    lazy = true,
    opts = {},
    init = function()
      package.preload["nvim-web-devicons"] = function()
        local Icons = require("mini.icons")
        local ret = {}
        package.loaded["nvim-web-devicons"] = ret
        Icons.mock_nvim_web_devicons()

        local function get(cat)
          local all = {}
          for _, name in ipairs(Icons.list(cat)) do
            local icon, color = ret.get_icon_color(cat == "file" and name, cat == "extension" and name)
            all[name] = { icon = icon, color = color }
          end
          return all
        end

        ret.get_icons_by_extension = function()
          return get("extension")
        end

        ret.get_icons_by_filename = function()
          return get("file")
        end

        ret.get_icons = function()
          return vim.tbl_extend("force", get("file"), get("extension"))
        end
        return ret
      end
    end,
  },
echasnovski commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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 :facepalm:

0x0003 commented 2 days ago

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 :confused:

wroyca commented 2 days ago

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 :partying_face:

xarvex commented 2 days ago

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 commented 1 day ago

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 commented 1 day ago

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:


@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 commented 1 day ago

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

echasnovski commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

preload is only useful when:

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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 commented 1 day ago

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

wenjinnn commented 1 day ago

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,
      },
    })
xarvex commented 1 day ago

@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.

Okay, so the issue here is on the Telescope side only passing the extension, got it. Appreciate the looking into it, fantastic module yet again!

echasnovski commented 23 hours ago

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

@Allaman, it doesn't look like 'mini.icons' or 'nvim-lualine' issue. The "get_icon is nil value" error means that mocking was not actually done. There were changes in how 'nvim-web-devicons' mocking is done which breaks the code snippet you use in init filed of lazy specification. Take a look at how this now done in LazyVim, which should work.


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

@Joao-Queiroga, yes but not very straightforward. You can create custom highlight group with desired foreground and then use it inside customization. Something like this:

vim.api.nvim_set_hl(0, 'MyGreen', { fg = '#00ff00' })
require('mini.icons').setup({
  default = { file = { hl = 'MyGreen' } },
})

Okay, so the issue here is on the Telescope side only passing the extension, got it.

Not exactly, as it turned out. It supplies both filename and extension, from which 'mini.icons' mock chooses the second one. After thinking about it, I think I might switch to prefer the first one.


@mehalter, I'll take a look if mocking 'lspkind.nvim' is feasible. If it has a lot of API surface area, then it probably won't happen.

alekspickle commented 20 hours ago

tried it instead of nvim-dev-webicons and all icons disappeared :( . even on mini.files. Not sure what is wrong with or without MiniIcons.mock_nvim_web_devicons() makes no difference - all icons are just Screenshot_20240705_135622

patelharsh9797 commented 20 hours ago

Anyway to set custom icon for docker-compose file? like using regex or something and than use the same icon and color from the Dockerfile. image

Allaman commented 18 hours ago

Anyway to set custom icon for docker-compose file? like using regex or something and than use the same icon and color from the Dockerfile. image

Keep in mind that they introduced compose.y[a]ml

echasnovski commented 16 hours ago

tried it instead of nvim-dev-webicons and all icons disappeared :( . even on mini.files. Not sure what is wrong

@alekspickle, adding single require('mini.icons').setup() (with installed 'mini.nvim' or 'mini.icons') should make it work for 'mini.files' (make sure to update it if you are using standalone repo).

To make it work with plugins which (currently) rely only on 'nvim-web-devicons', also add extra MiniIcons.mock_nvim_web_devicons() call afterwards (like described in help).

Anyway to set custom icon for docker-compose file? like using regex or something and than use the same icon and color from the Dockerfile.

@patelharsh9797, I am afraid, the answer currently is to only add custom file names to match exactly. So something like this:

local icons = require('mini.icons')
local get_icon_tbl = function(category, name)
  local icon, hl = icons.get(category, name)
  return { glyph = icon, hl = hl }
end
local dockerfile_icon_tbl = get_icon_tbl('filetype', 'dockerfile')
icons.setup({
  file = {
    ['docker-compose.yml'] = dockerfile_icon_tbl,
    ['docker-compose.dev.yml'] = dockerfile_icon_tbl,
  },
})

The "regex or something" approach would have been to define custom filetype matching rules via vim.filetype.add() (so that some 'yml' files are detected as having 'dockerfile' filetype), but currently this would have no effect as 'yml' and 'yaml' are manually tracked extensions (and thus have preference over vim.filetype.match()). At the moment I think this is the right decision.

danilax999 commented 16 hours ago

Hi! There is an issue with default filetype icons that are defined not in lower case (e.g. Neogit, NvimTree, Trouble). Filetype icons should probably not be case-insensitive, as vim filetypes are case-sensitive, or all default filetype icons should be lowercased.

echasnovski commented 15 hours ago

@xarvex, @SebasF1349: Telescope (and all others using require('nvim-web-devicons').get_icon() with both filename and extension) should now work better with mock_nvim_web_devicons(). Prioritizing name argument instead of ext indeed seems like a better alternative on practice.


@alekspickle, as you've edited your comment (which I could have easily missed, by the way, if not the new screenshot), the issue seems like not up to date font. The 'mini.icons' module specifically requires font compatible with Nerd Fonts 3.0.0+. Updating (or using different but up to date) font should help here.


@danilax999, I already had that fixed locally before reading your comment :) This also did not work with "enumMember" and "typeParameter" from "lsp".

Although brief search through documentation had nothing on whether 'filetype' option should be treated regarding letter casing, I don't think supporting case sensitive match is a good idea. This will only matter if there are two different filetypes which are equal when converted to lowercase (like 'NvimTree' and 'nvimtree'). I do not want to encourage using this type of filetypes.

Should be fixed on latest main.

miszo commented 14 hours ago

@echasnovski thank you for the great plugin! 🙇

Do you intend to support different icons for test files? The nvim-web-devicons has that covered, and it helps a lot with finding the tests files on the list. https://github.com/nvim-tree/nvim-web-devicons/blob/master/lua/nvim-web-devicons/icons-default.lua#L2957-L2980

echasnovski commented 14 hours ago

@echasnovski thank you for the great plugin! 🙇

🙏

Do you intend to support different icons for test files? The nvim-web-devicons has that covered, and it helps a lot with finding the tests files on the list. https://github.com/nvim-tree/nvim-web-devicons/blob/master/lua/nvim-web-devicons/icons-default.lua#L2957-L2980

Sorry, but this type of language specific icons won't have support out of the box.

My best suggestion is to add those files in your setup call in "file" category.

mehalter commented 13 hours ago

Sometimes I want to get icons with some sort of priority. Such as get the icon for a filename and if that doesn't exist, get the icon for the filetype. With the current implementation it can be tricky to know if the get function failed. It would be nice to either be able to ask for an icon with a priority list of types and the parameters for each type. Or, more generalize-able would be to be able to skip the default return. Then I can simply check if it icon was returned or not before going to the next source I care about

mehalter commented 13 hours ago

@echasnovski it looks like create_default_hl isn't being called on ColorSchemeautocommand event, so changing colorschemes can break the colors if they clear the space.

Edit: opened a PR to help on this one, https://github.com/echasnovski/mini.nvim/pull/1016

miszo commented 13 hours ago

Sorry, but this type of language specific icons won't have support out of the box.

My best suggestion is to add those files in your setup call in "file" category.

Thanks for the suggestion. I've achieved what I wanted with the "extension" category. Here's my setup.

  {
    'echasnovski/mini.icons',
    lazy = true,
    config = function()
      local test_icon = ''
      local js_table = { glyph = test_icon, hl = 'MiniIconsYellow' }
      local jsx_table = { glyph = test_icon, hl = 'MiniIconsAzure' }
      local ts_table = { glyph = test_icon, hl = 'MiniIconsAzure' }
      local tsx_table = { glyph = test_icon, hl = 'MiniIconsBlue' }
      require('mini.icons').setup({
        extension = {
          ['test.js'] = js_table,
          ['test.jsx'] = jsx_table,
          ['test.ts'] = ts_table,
          ['test.tsx'] = tsx_table,
          ['spec.js'] = js_table,
          ['spec.jsx'] = jsx_table,
          ['spec.ts'] = ts_table,
          ['spec.tsx'] = tsx_table,
          ['cy.js'] = js_table,
          ['cy.jsx'] = jsx_table,
          ['cy.ts'] = ts_table,
          ['cy.tsx'] = tsx_table,
        },
      })
    end,
  },

And this is the outcome CleanShot 2024-07-05 at 17 33 08@2x

echasnovski commented 12 hours ago

Sometimes I want to get icons with some sort of priority. Such as get the icon for a filename and if that doesn't exist, get the icon for the filetype. With the current implementation it can be tricky to know if the get function failed. It would be nice to either be able to ask for an icon with a priority list of types and the parameters for each type. Or, more generalize-able would be to be able to skip the default return. Then I can simply check if it icon was returned or not before going to the next source I care about

Always returning an icon is an intentional design choice. Here are some parts of the reasoning:

My current best suggestion is to compare to default glyph of category. They are specifically chosen to be different from all others category glyphs (and if user overrides them to something already present, then something bad happening is justified).

Or for the case of "file" -> "filetype" falling back, consider using vim.filetype.add().

@echasnovski it looks like create_default_hl isn't being called on ColorSchemeautocommand event, so changing colorschemes can break the colors if they clear the space.

Answered in PR.

mehalter commented 12 hours ago

Yeah the main reason for falling back is in non file buffers. For instance in the statusline I would want it to default to using the file if it exists. I suppose comparing against the default is a fine approach. That's what I'm doing currently it just seems particularly "hackish" since there is no semantics in an icon providing plugin for an icon not existing. For now it is fine to just use this "hack" comparing against the default icon for each category. Thanks!

mehalter commented 12 hours ago

Thanks for the fast response in the PR! Definitely still curious about the stance in the PR regarding the default highlight setting since :hi clear is used by every single colorscheme to make sure it doesn't coincide with every other colorscheme 🤔 It also seems completely standard for plugins (including the other modules in mini.nvim) to reset their default highlights on the ColorScheme event

Parsifa1 commented 12 hours ago

It seems that this function of for lualine does not work properly when use mini.icons

image image image

mehalter commented 12 hours ago

This looks like you are probably setting minifiles file name. Technically mini.icons should be able to resolve an icon because there is a filetype icon there but it's probably looking it up by name first and getting the default file icon then isn't knowing that it should re-check for the filetype because the get function with the filename returns the default file icon. As state above the developer is not open to changing this behavior for the reasons listed above so lualine either needs to just add direct mini.icons support or that is just an icon that will exist for all non-file buffers it seems

echasnovski commented 12 hours ago

Yeah the main reason for falling back is in non file buffers. For instance in the statusline I would want it to default to using the file if it exists.

@mehalter, if the buffer is a "non file buffer", then it does not have a file associated with it. I.e. vim.api.nvim_buf_get_name(buf) returns empty string. I'd personally condition on that.

It seems that this function of for lualine does not work properly when use mini.icons

@Parsifa1, I am not sure what is broken as I don't use 'lualine.nvim'. Is there a reason you supply icon as a part of the name?

mehalter commented 12 hours ago

The reason previously was that nvim-web-devicons doesn't provide those icons and it was a hack to get those icons in there for those filetypes rather that adding the filetype icons to nvim-web-devicons configuration.

The reason the first icon is the default file icon rather than the mini files icon that's in mini.icons is because it always resolves an icon from the filename. nvim-web-devicons returns nil which lualine uses to re-check for the filetype icon.

echasnovski commented 12 hours ago

The reason previously was that nvim-web-devicons doesn't provide those icons and it was a hack to get those icons in there for those filetypes rather that adding the filetype icons to nvim-web-devicons configuration.

Ok. But now there is support for filetype in 'nvim-web-devicons'. I am not sure I want to adjust core logic of 'mini.icons' based on the outdated assumptions about 'nvim-web-devicons'.

Parsifa1 commented 12 hours ago

The reason previously was that nvim-web-devicons doesn't provide those icons and it was a hack to get those icons in there for those filetypes rather that adding the filetype icons to nvim-web-devicons configuration.

The reason the first icon is the default file icon rather than the mini files icon that's in mini.icons is because it always resolves an icon from the filename. nvim-web-devicons returns nil which lualine uses to re-check for the filetype icon.

ok. i think wait for lualine to support mini.icons is better idea :)

mehalter commented 12 hours ago

Ok. But now there is support for filetype in 'nvim-web-devicons'. I am not sure I want to adjust core logic of 'mini.icons' based on the outdated assumptions about 'nvim-web-devicons'.

Yeah, I suppose this also could be considered a "bug" in the nvim-web-devicons mocking. Maybe there should be a check in there that checks for the default and returns nil to match the devicons API. Here is the code in question for Lualine:

https://github.com/nvim-lualine/lualine.nvim/blob/master/lua/lualine/components/filetype.lua#L34-L37

Like all of the other statusline providers, it first checks for the icon with the filename and then if an icon doesn't exist then it falls back to the filetype. This provides the best support for file and non-file buffers.