feat: add icon.provider option to components for full control

[mehalter]

This supersedes #1510

Previously I looked at adding direct support for mini.icons into the code base. @miversen33 made a fantastic point that it seems like icon providing is a simple pattern that can grow arbitrarily and it might make sense to make it configurable by the user to provide their own icon provider for components. This adds a new section to the common section of the components to utilize an icon provider for resolving icons based on a node/state.

I took a look at the document_symbols component and I'm honestly not sure what the best approach for that part is. Should we try to unify it with the component's icon provider and have it override it to an icon provider of LSP kinds or should we make a new config option specifically for that component that has a similar pattern?

Let me know what you think of this, it basically sets up the current nvim-web-devicons support as a default icon provider for components.

Here is an example of using the new feature where the user uses mini.icons:

---@type LazySpec
return {
  "nvim-neo-tree/neo-tree.nvim",
  dependencies = {
    { "echasnovski/mini.icons", opts = {} } -- add mini.icons
  },
  opts = {
    default_component_configs = {
      icon = {
        provider = function(icon, node) -- setup a custom icon provider
          local text, hl
          local mini_icons = require "mini.icons"
          if node.type == "file" then -- if it's a file, set the text/hl
            text, hl = mini_icons.get("file", node.name)
          elseif node.type == "directory" then -- get directory icons
            text, hl = mini_icons.get("directory", node.name)
            -- only set the icon text if it is not expanded
            if node:is_expanded() then text = nil end
          end

          -- set the icon text/highlight only if it exists
          if text then icon.text = text end
          if hl then icon.highlight = hl end
        end,
      },
    },
  },
}

[miversen33]

I have not tested this but it passes the eye test for me. Thanks for this (and the previous #1510 ) PRs :)

I will leave it up to @pysan3 for a final decision though :)

[mehalter]

@miversen33 what do you think about the path forward for adding support through to the document_symbols or other components? I'm curious if there is a path through to more generalization as well especially while ensuring no breaking changes

[miversen33]

IMO we put this in the default component object (as you did) and leave it up to other components to use this. That said, its probably worth retrofitting the internal providers (such as document_symbols, git, etc) to use this. But we have no control over 3rd party providers (such as netman) and therefore the best we can do is keep it at a high level and default. This way if a provider doesn't implement handling for icons, then the default component does it for them via this.

[mehalter]

Yeah that's exactly the path forward that I was thinking. I guess it's more that upon a cursory glance I didn't see the best way to retrofit it. It might be that someone with more experience with the code base can go back and retrofit the internal providers if this is decided to be a good direction.

Thanks so much for considering this change!

[mehalter]

@miversen33 I took some time to understand how the components work and I believe I have correctly retrofit the icon provider into the document_symbols source. I think this is the only source that uses a custom icon component.

Here is an updated snippet of using icon.provider to use mini.icons now with LSP kinds along with the file/directories:

---@type LazySpec
return {
  "nvim-neo-tree/neo-tree.nvim",
  dependencies = {
    { "echasnovski/mini.icons", opts = {} }, -- add mini.icons
  },
  opts = {
    default_component_configs = {
      icon = {
        provider = function(icon, node) -- setup a custom icon provider
          local text, hl
          local mini_icons = require("mini.icons")
          if node.type == "file" then -- if it's a file, set the text/hl
            text, hl = mini_icons.get("file", node.name)
          elseif node.type == "directory" then -- get directory icons
            text, hl = mini_icons.get("directory", node.name)
            -- only set the icon text if it is not expanded
            if node:is_expanded() then
              text = nil
            end
          elseif node.type == "symbol" then
            local kind = vim.tbl_get(node, "extra", "kind", "name")
            if kind then
              text, hl = mini_icons.get("lsp", kind)
            end
          end

          -- set the icon text/highlight only if it exists
          if text then
            icon.text = text
          end
          if hl then
            icon.highlight = hl
          end
        end,
      },
    },
  },
}

[mehalter]

Actually I realized I went about this incorrectly. It should be a provider in the kind_icon component actually.

Here is an updated test snippet:

---@type LazySpec
return {
  "nvim-neo-tree/neo-tree.nvim",
  dependencies = {
    { "echasnovski/mini.icons", opts = {} }, -- add mini.icons
  },
  opts = {
    default_component_configs = {
      icon = {
        provider = function(icon, node) -- setup a custom icon provider
          local text, hl
          local mini_icons = require("mini.icons")
          if node.type == "file" then -- if it's a file, set the text/hl
            text, hl = mini_icons.get("file", node.name)
          elseif node.type == "directory" then -- get directory icons
            text, hl = mini_icons.get("directory", node.name)
            -- only set the icon text if it is not expanded
            if node:is_expanded() then
              text = nil
            end
          end

          -- set the icon text/highlight only if it exists
          if text then
            icon.text = text
          end
          if hl then
            icon.highlight = hl
          end
        end,
      },
      kind_icon = {
        provider = function(icon, node)
          local mini_icons = require("mini.icons")
          icon.text, icon.highlight = mini_icons.get("lsp", node.extra.kind.name)
        end,
      },
    },
  },
}

[mehalter]

Any updates on this? :)

[cseickel]

Hey @mehalter sorry to let this sit for so long. I had stepped away from the project but it looks like no ones at the wheel right now. @miversen33 it looks you did some work on this, could you approve and merge? I think you have permission.

[miversen33]

The code looks good, though I haven't tested it.
@cseickel I do not have merge permissions for neo-tree, though I am happy to perform this merge (after a bit of testing since it would be my name on the blame) if you want.

That said, I am not in a spot to be able to properly care for this project. I know you weren't asking me to, I just wanted to head that off lol

[cseickel]

The code looks good, though I haven't tested it. @cseickel I do not have merge permissions for neo-tree, though I am happy to perform this merge (after a bit of testing since it would be my name on the blame) if you want.

I just made you an admin, please do merge because I know I won't have the time to test or properly review myself.

That said, I am not in a spot to be able to properly care for this project. I know you weren't asking me to, I just wanted to head that off lol

You know I was thinking it! Either way, I do trust you to do what is best and you do have full control now, so feel free to jump in and promote others or do whatever needs to be done if I become unresponsive.

[miversen33]

I'm going to merge this in. It worked well when I poked it, the code looked good and the documentation is good :) Thanks for this @mehalter !

[miversen33]

LGTM

[mehalter]

Thanks so much everyone here for reviewing! Glad to contribute!

[mehalter]

@miversen33 do you think it would be a good idea to tag a new release? It's been a few months since the last one and this feature is quite useful and so is the pop up title fix that was merged in a few months ago :)

[miversen33]

Frankly I thought the release schedule was automated lol. I would like to wait for @pysan3 but if we don't hear from the by the end of next week, I am fine doing a release next Friday

[mehalter]

Sounds great, thanks again for your help!

[cseickel]

I just made a new release.

[Zeioth]

Thank you for the PR. Closing #1589