Foggalong / hardcode-fixer

Fixes Hardcoded Icons
GNU General Public License v3.0
287 stars 49 forks source link

Icon name issue #285

Open leoheck opened 7 years ago

leoheck commented 7 years ago

Some time ago I had added these 2 lines in tofix.csv:

Layout Editor,layout,/usr/share/icons/hicolor/48x48/apps/layout.png,layout
Layout Editor,layout,layout.png,layout

However, this is stupid. Why it is needed to add multiples lines regarding the image referenced by Icon=. The script should replace Icon=.* by Icon=<new_icon_name>. Doesn't matter where the old icon was.

Considering this, today I installed Layout Editor in a new machine using .deb file instead of using apt. In this new installation method, the Icon is pointing to a different path. So I have to change tofix.csv again adding a new line for the same thing.

Name=Layout Editor
GenericName=Layout Editor
Icon=/opt/layout/install/layout

Considering you are using this for undo purposes, the tofix.sh could save old paths in a file like ~/.tofix.oldpaths.

What do you think? This can avoid changing the same thinks always.

Foggalong commented 7 years ago

The idea behind doing it the current way was so that we weren't unnecessarily fixing icons if they didn't need to be fixed. To do otherwise would be inconsequential for an app developer to fix it on their end, because the script will change the value either way. I'm gonna have a think about this though and see if it can't be done in a smarter way.

bilelmoussaoui commented 7 years ago

@Foggalong You can detect if an icon is hardcoded easily, Once you got the Icon value from the desktop file; just check if it's a path or has an extension

Foggalong commented 7 years ago

@bil-elmoussaoui Good idea! Will definitely implement that in the rewrite :)

leoheck commented 7 years ago

Hahaha, cool. Let us know if you need some help.

leoheck commented 7 years ago

I was just thinking now. We can create a repository for desktop files instead of just fixing the icons. We can provide well-organized desktop files with extra improvements (e.g. fixing app names and missing sections/tags) for each file. We can just save the old file in place adding a string to the filename (e.g. nautilus.desktop.bkp). What do you think @Foggalong? In this way, we do not need a fix.csv, we just need files and a way to backup these files before to add our new standardized one to the user/system folder.

bilelmoussaoui commented 7 years ago

@leoheck it's not the best solution as packages doesn't follow any guidelines; The Exec attributes isn't always the same....

leoheck commented 7 years ago

Hm, I see. But it is still possible to:

  1. make a .desktop.bkp backup
  2. put the standard .desktop in place
  3. replace the Exec= from the original

This is still a good approach.

bilelmoussaoui commented 7 years ago

@leoheck not really, as it will involve more energy and time to create all those desktop files; fix them update them whenever something changes upstream... It's better to keep the purpose of this script as simple as possible, a database of hardcoded icon names, and new icon names. Run the tests, find a match, fix it! that's all... even for backups, I don't even think it's necessary if things are done correctly (copying the original icon to hicolor theme in order to keep the icons looking fine in other themes that do not support it).

leoheck commented 7 years ago

Well, you are right. But I still think that multi-entries for each icon/app is a waste of energy which should be fixed.

Foggalong commented 7 years ago

@leoheck That will be fixed, don't worry :)

MarekAG commented 7 years ago

@Foggalong I think that relying on app name is better approach. This will fix #149 and also e.g. my (closed) #288. As @bil-elmoussaoui said we can at first check if the .desktop "Name=" equals app name in tofix.csv (this should be rewriten to contain app names from .desktop files beside human readable ones). Then check "Icon=" value " it's a path or has an extension", if answer is "yes" then use proper icon from tofix.csv.

What do you think about this approach? Am I missing something?

Foggalong commented 7 years ago

I was literally doing some work on the rewrite when your notification came through. It doesn't work in quite the same way you suggested, but it will still fix this issue #220, #178, and #149. There'll hopefully be an update posted to all these issues calling for testing shortly.