nvim-tree / nvim-web-devicons

lua `fork` of vim-web-devicons for neovim
MIT License
1.93k stars 182 forks source link

refactor: rewrite generator #414

Open futsuuu opened 3 months ago

futsuuu commented 3 months ago

Currently, icons are defined separately one by one, so it takes time to add or change icons for the same type of file. For example, if we want to add more icons of same format, we have to repeat the copy and paste for each extension. After a while, if we want to change the colors of these icons, we will have to find all of them and replace the colors correctly.

This PR rewrites the generator to fix these probrem.

before:

+ [".gitattributes"] = { icon = ".", color = "#......", name = "GitAttributes" },
  [".gitconfig"] = { icon = ".", color = "#......", name = "GitConfig" },
+ [".gitkeep"] = { icon = ".", color = "#......", name = "GitKeep" },
+ [".gitmodules"] = { icon = ".", color = "#......", name = "GitModules" },

after:

- Git = { icon = "seti-git", color = "#......", file = { ".gitconfig" } },
+ Git = { icon = "seti-git", color = "#......", file = { ".gitattributes", ".gitconfig", ".gitkeep", ".gitmodules" } },
alex-courtis commented 3 months ago

This looks great! It works and will be a dev time savings.

This is a refactor of the existing functionality: a major refactor of the source files. That adds value.

This is, however, a nearly full rewrite of the generator. Unfortunately I can't justify such a large change, with the additional complexity and maintenance it requires.

alex-courtis commented 3 months ago

All is not lost!

We have a major generation change planned: use the css classes rather than icons: #192 which would allow us to create icon sets, starting with seti #391

This css class change would marry beautifully with this refactor. Are you up for the challenge @futsuuu ?

alex-courtis commented 3 months ago

We will help you make and test this change. See https://github.com/nvim-tree/nvim-web-devicons/pull/376#issuecomment-1890843503 for a proof of concept gist to extract the icons from nerd fonts.

futsuuu commented 3 months ago

Sure! But I can't imagine how you want to create icon sets, could you tell me?

futsuuu commented 3 months ago

This is, however, a nearly full rewrite of the generator. Unfortunately I can't justify such a large change, with the additional complexity and maintenance it requires.

I understand this, but the definition and the generated should be separate files for maintenance.

alex-courtis commented 3 months ago

I understand this, but the definition and the generated should be separate files for maintenance.

Absolutely. This change and the addition of css class names does justify the change.

I was being too dramatic in my language...

alex-courtis commented 3 months ago

Sure! But I can't imagine how you want to create icon sets, could you tell me?

Let's build one default set first, then we can decide about icon sets. It may be best to add them after this change.

alex-courtis commented 3 months ago

I can build a glyphnames.json + new css class mappings -> icons_by_filename-like that you could work with.

My time is, however, limited so I cannot guarantee timely delivery.

alex-courtis commented 3 months ago

Here are the changes to generate the icons from css classes: #418

It is clunky and tightly coupled right now. Integration with this rewrite will result in an elegant solution.

hasecilu commented 2 months ago

Me at #433

Would be possible to create groups, for example create a single icon entry and use it various times for different file extensions to simplify the code?

This would be very useful because I reuse a lot of icons