-
Notifications
You must be signed in to change notification settings - Fork 15
Add duplicate icons that were already created #197
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
Fixes eclipse-platform#192. Fixes eclipse-platform#191. Fixes eclipse-platform#190. Fixes eclipse-platform#189. Fixes eclipse-platform#188. Fixes eclipse-platform#187. Fixes eclipse-platform#186. Fixes eclipse-platform#185. Fixes eclipse-platform#184. Fixes eclipse-platform#183. Fixes eclipse-platform#182. Fixes eclipse-platform#181. Fixes eclipse-platform#180. Fixes eclipse-platform#179. Fixes eclipse-platform#178. Fixes eclipse-platform#177. Fixes eclipse-platform#176. Fixes eclipse-platform#175. Fixes eclipse-platform#174. Fixes eclipse-platform#173. Fixes eclipse-platform#172.
7955331 to
4bbb4ca
Compare
Instead of adding duplicates, one might want to use links instead: https://stackoverflow.com/questions/954560/how-does-git-handle-symbolic-links |
|
It's probably not idea to assume that works well on Windows... |
See https://stackoverflow.com/questions/5917249/git-symbolic-links-in-windows Its a litte more setup, but we already require some special setup for SWT git-lfs as well, given that this repo is not being used by much developers it seems better than to repeat the C&P style of previous days. If that's not sufficient enough, I would propose to do it at least automatic by a (maven build) script on demand during the build instead of polluting the repository. I think I suggested it already earlier that a more semantic structure for icons would be much better... it then can be copied to whatever structure we want later on. |
|
@Michael5601: Should I merge these? Should I or @jasmin261098 do a detailed review on them? |
See above, I don't think this is a useful pattern, it makes the whole thing completely unmanageable. |
I don't quite see the benefits of your proposal yet. Maybe I do not understand it fully yet. I am open to change if you want to elaborate it more. The folder in which all dual tone icons will be saved resembles the exact location of every icon in all eclipse repositories. Why should this be less manageable than the repositories itself? This approach might be the naive solution but it is also very simple, makes sure we don't forget any icons, gives us the ability to view the progress of the project, allows simple refactoring of any existing icons (also programatically e.g. changing colors) and makes migration of the icons to eclipse itself later on very easy. The last point is especially important for demonstration purposes as this project may take a while. Your proposal adds complexity and is also the wrong end to begin with in my opinion. I would rather have improvements in the duplicate icon situation in eclipse itself than on this dual tone icon project. |
If the concerns of @laeubi are settled we can merge in my opinion. I only added all icons, we already designed to the specific locations where their duplicates lie in eclipse. |
If the concerns of @laeubi are settled we can merge in my opinion. I only added all icons, we already designed to the specific locations where their duplicates lie in eclipse. So no need to review, if I made any errors, they will be found later on, when we update the progess of the project: #151 |
You raise a valid point about the current state of Eclipse repositories. Indeed, icon management there is already problematic (as evidenced by recent confusion with duplicate and forgotten updates to Eclipse Feature icons). However, this doesn't mean we should replicate those problems here - instead, this is an opportunity to establish better practices.
I agree that platform improvements are needed. However, if we simply replicate the existing structure here without addressing the duplication issue, we risk perpetuating the problem indefinitely. The platform improvements and this icon pack development can happen in parallel.
I understand your perspective on complexity. Let me clarify what I see as the trade-offs: Current approach (direct copying):
Proposed semantic approach:
Benefits of this approach:
I hope it is now a bit more clear, and I don't want to block the development but currently this does not feel to go in the right direction if we talk about developing "icon packs" in the future. |
Thank you for your detailed clarification. I can see your concerns way clearer now and I agree that the trade-offs are technical debt we should avoid in this early stage of the development. I did not have these points in mind before. I still have some concerns regarding the mapping file and semantic icon library:
Apart from my concerns I can see some advantages of your approach you did not mention yet:
So from my point of view we gain the advantages you and I mentioned but increase the overhead and thus development time for creating the icon pack. It is hard for me to decide if we should follow your approach. |
|
@Michael5601 Thanks for sharing your concerns. Let me address each point: 1. Naming Complexity for Similar Use Cases
This is precisely why a semantic style guide is valuable. The "next_nav" example you mentioned (#149) actually illustrates the importance of clear semantic distinctions:
Yes, semantic names may become specific, but that's a feature, not a bug. Clear, specific names:
2. 1:1 Mappings and Path Changes
While some icons may have 1:1 mappings initially, the semantic approach provides:
3. Maintenance Overhead
The overhead already exists - it's just invisible and distributed: Current (Hidden) Overhead:
Proposed (Visible) Overhead:
This isn't adding overhead - it's making existing overhead visible and manageable. 4. Traceability and Documentation
Yes, but consider the benefits:
5. Development Workflow
Manual copying seems easy but:
An automated script (one-time setup cost) provides:
Moving ForwardI strongly believe the initial setup investment will pay dividends. With modern AI tools, we can:
This isn't just about organizing icons - it's about creating a sustainable, maintainable icon system that scales with Eclipse's needs. The alternative is continuing with the current ad-hoc approach, which will only become more difficult to manage as the icon set grows. I could offer to help create a proof-of-concept showing how this could work with a subset of icons, I myself using a generator script in my application (where the script is actually some java code) that currently also includes the conversion from SVG>PNG (hopefully soon obsolete) and believe automation is simply a key in that area. |
|
@BeckerWdf what do you think? |
|
I do not know how the implementation plan for the icon packs on the platform side is planned. But we at Eclipse 4diac have an approach with enum and an fragment bundle with the icons and We have it now about 10 years and it helped us a lot to have a) different icon sets and b) change icons and icon locations without any change in the code. However I guess the enum based approach is a bit to rigid for the platform. But as part of this implementation there is also an own icon url for |
Can you explain that in a bit more details? |
Sure. I tried to find the original Eclipse Magazine article, but somehow I either lost it or just mix stuff up. But I can explain how we implemented the concept in 4diac IDE. The core part are three classes:
The images themselves are provided by a fragment bundle which provides the With this infrastructure you write in Java code: And in a The great advantage for us as RCP is that we have one single place for all of our icons and one could easily provide a different set of icons by providing a different fragment bundle. Before that we also had the problem that we had icons doubled. Different icons for the same thing. The drawback is this approach is that in its current implementation it is rather rigid. All icons have to be in one single enum. This works good for us as RCP but for the Platform a more extensible approach would be needed. What I definitely see for the platform is the own URL as this allows to decouple icon file location from icon usage in plugin.xmls, and E4 models. |
@laeubi has a point here. So maybe could fix a part of the problem here in the icon pack without the need to touch the "productive" bundles.
With this we would not duplicate files and with the mapping file we still can automate the exchange from old to new files. |
I agree on the proposal of @laeubi and the workflow steps you provided. On the weekend I will propose a PR that adresses this, then we can further refine this approach. |
Fixes #192.
Fixes #191.
Fixes #190.
Fixes #189.
Fixes #188.
Fixes #187.
Fixes #186.
Fixes #185.
Fixes #184.
Fixes #183.
Fixes #182.
Fixes #181.
Fixes #180.
Fixes #179.
Fixes #178.
Fixes #177.
Fixes #176.
Fixes #175.
Fixes #174.
Fixes #173.
Fixes #172.