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

refactor: rewrite generator #414

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

futsuuu
Copy link
Contributor

@futsuuu futsuuu commented Mar 9, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

We will help you make and test this change. See #376 (comment) for a proof of concept gist to extract the icons from nerd fonts.

@futsuuu
Copy link
Contributor Author

futsuuu commented Mar 10, 2024

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

@futsuuu
Copy link
Contributor Author

futsuuu commented Mar 10, 2024

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.

@futsuuu futsuuu changed the title refactor: improve generator refactor: rewrite generator Mar 10, 2024
@alex-courtis
Copy link
Member

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
Copy link
Member

alex-courtis commented Mar 14, 2024

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
Copy link
Member

alex-courtis commented Mar 14, 2024

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
Copy link
Member

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
Copy link
Collaborator

hasecilu commented Apr 5, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants