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

Update makefile for pixmap #126

Closed
wants to merge 1 commit into from
Closed

Update makefile for pixmap #126

wants to merge 1 commit into from

Conversation

Pickle
Copy link
Collaborator

@Pickle Pickle commented Aug 29, 2019

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

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

postiffm commented Aug 29, 2019

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.

@postiffm postiffm closed this Aug 29, 2019
@rluzynski
Copy link
Collaborator

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

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

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.

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