search

postiffm / bibledit-desktop

Desktop version of Bibledit
GNU General Public License v3.0
4 stars 6 forks source link

Update makefile for pixmap #126

Closed Pickle closed 5 years ago

Pickle commented 5 years ago

The changes readd assigments that will install the xpm icon into the pixmaps folder. This fixes no icon appearing for the menu item.

postiffm commented 5 years ago

This PR is now included in the codebase, commit 4f9f304. Thanks @Pickle for your contributions!

Note: beware that I am often doing forced pushes on the gtk3 branch because I am rebasing it against the master (gtk2) branch. We want a clean history after all the gtk3 branch development is done, and this helps to do that.

rluzynski commented 5 years ago

This pull request was supposed to fix the bug described in this disuccion and introduced by my pull request #121 (commit: Do not use and do not distribute the old icon.) I have reproduced the issue as described by @Pickle and I also have found a better solution. The missing piece is that after install the following command should be executed:

gtk-update-icon-cache /usr/share/icons/hicolor

See the complete documentation.

However, it is controversial whether we should add this command to make install script. Yes, it is necessary if we install Bibledit systemwide from the source code. However, this is not how applications should be installed by users nowadays. The recommended way is to use packages and it's the job of the package scripts to execute this command. Additionally, some package managers and some distributions have a feature to trigger this command automatically whenever icons are added/changed/removed which makes executing this command obsolete. See the discussion about removing explicit execution of gtk-update-icon-cache.

Also please note that the old bibledit-desktop.xpm icon whose use this patch restores is smaller (48x48) and has lower quality than the PNG icons (up to 96x96) which I had introduced by another patch.

My suggestion is to revert or rather just drop the commit introduced by this pull request and either add the gtk-update-icon-cache to Makefile or (maybe better) explain in the documentation where this should be executed and why not.

postiffm commented 5 years ago

@rluzynski Not sure if this should be closed now or not...I would like to do everything the "standard" way so that other users will be accustomed to it. BUT, if anyone wants to build from source instead of getting the program through a package (say they need the latest changes before the package managers provide those changes) then it would be nice to have a way that the user can do a full install and have everything work properly.

rluzynski commented 5 years ago

My concerns are addressed now by the pull request #128. And yes, the script detects whether it is ran by a user willing to install the newest version from source or another script building a distro package and supports both cases. My comment above predates the pull request, you can ignore it now.