-
Notifications
You must be signed in to change notification settings - Fork 200
refactor: rewrite generator #414
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
Conversation
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. |
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. |
Sure! But I can't imagine how you want to create icon sets, could you tell me? |
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... |
Let's build one default set first, then we can decide about icon sets. It may be best to add them after this change. |
I can build a My time is, however, limited so I cannot guarantee timely delivery. |
32c27b1
to
c22708c
Compare
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. |
Me at #433
This would be very useful because I reuse a lot of icons |
Handled by #550 |
No, it's not, unfortunately. This PR aimed to solve the problem of having to define the same icon table repeatedly when assigning to multiple targets; Instead of defining list of targets with single icon definition. Some sort of generator like this would be very valuable, be it what was proposed in this PR, or better yet, autogenerated from css classes (or similar). Might be worth reopening although looks like @futsuuu has lost their interest. |
Ah I see, that would be useful for the repeated ones. Maybe with CSS ? |
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:
after: