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

Correct linux menu launch shortcut #124

Merged
merged 1 commit into from
Aug 25, 2019
Merged

Correct linux menu launch shortcut #124

merged 1 commit into from
Aug 25, 2019

Conversation

Pickle
Copy link
Collaborator

@Pickle Pickle commented Aug 24, 2019

The linux gtk menu launcher was incorrectly set to exec bibledit-gtk it should be bibledit-desktop.

The icon entry also did not refer to an actual icon, this commit change it to the xpm icon.
The commit also changes references to bibledit-gtk. Readme and changelog reference were not changed.

… It should be bibledit-desktop.

The icon entry also did not refer to an actual icon, this commit change it to the xpm icon.
The commit also changes references to bibledit-gtk. Readme and changelog reference were not changed.
@Pickle Pickle requested a review from postiffm August 24, 2019 21:23
@Pickle Pickle added the bug label Aug 24, 2019
@postiffm
Copy link
Owner

This touches some stuff we mod in the gtk3 branch. But if it helps you here, I'll deal with those potential conflicts later.

@postiffm postiffm merged commit 03d0040 into master Aug 25, 2019
@postiffm
Copy link
Owner

@Pickle for tidiness of git log --oneline, please put a single-line description of your commit, and then a blank and then after that you can put any number of lines of arbitrary length for further description. Thanks for this PR.

postiffm added a commit that referenced this pull request Aug 25, 2019
    This includes the *.desktop file rename and corrections.

    Closes #115

    MAP added 8/24/2019: Resolve small conflict introduced by PR #124
@LAfricain
Copy link
Contributor

Does this merge at least temporarily fix this issue for linux: #111?

@Pickle
Copy link
Collaborator Author

Pickle commented Aug 26, 2019

@postiffm i will make future commits like you ask

@LAfricain im not sure it is exactly the same problem.
What I did was run sudo make install. I saw i had the menu entry so it at least copied the desktop file. But i found it would not launch the application and it did not have any icon.

@Pickle Pickle deleted the fixmenulaunch branch August 26, 2019 12:34
@postiffm
Copy link
Owner

I thought we already fixed this by pulling some gtk3 branch commits into the master branch. But I haven't independently verified if that is correct.

@Pickle
Copy link
Collaborator Author

Pickle commented Aug 27, 2019

I checkout the gtk3 branch last night and im not sure my icon changes are correct.
GTK3 has the same icon problem and did not work adding the xpm extension.
I looked at another application and its desktop file referenced a icon with no extension, so the original icon setting could be correct.
What i think is missing is the make install is not copying a png to the pix folder.
I will test this out tonight and if it works I think I just need to find where in the makefile to copy a png to the right folder.

@rluzynski
Copy link
Collaborator

Excellent, I saw the commit and this is exactly the port of the changes I made in the gtk3 branch to master. Yes, it should fix the problem @LAfricain mentioned in #111 for Linux with GTK2 (but not for Windows).

@Pickle:

I checkout the gtk3 branch last night and im not sure my icon changes are correct.
GTK3 has the same icon problem and did not work adding the xpm extension. [...]

What problem are you talking about? Are you sure you have the latest version of gtk3 branch? I think I verified that all *.png files are copied to the correct destinations. What may be missing is the launch of update-desktop-database command line utility. I don't recommend adding it explicitly because AFAIK this should be a job of the distribution packages.

@Pickle
Copy link
Collaborator Author

Pickle commented Aug 27, 2019

@rluzynski
master checked out.
ran make uninstall
checked out gtk3 branch and built
make install
went to the menu and i have a working menu entry that launches proper but has no icon.

I have beyond compare installed and i was curious how their desktop file looked and i found it was using just bcomp with no extension. So then i wondered where they put their icon image. I found one in the /usr/share/pix folder (need to confirm full path), but there was not one for bible edit in this location.
So my test will be to copy a bible edit png to the location and see if I get the icon showing up.
Maybe the location is specific to the distro? Im currently running linux mint with xfce.

@Pickle
Copy link
Collaborator Author

Pickle commented Aug 27, 2019

ok i ran the test,
on the gtk3 branch i need to copy bibledit-desktop.xpm to /usr/share/pixmaps. No changes are needed to the desktop file. But once the file is there the image shows up in my menu.

@Pickle
Copy link
Collaborator Author

Pickle commented Aug 28, 2019

I pretty sure I have it figured out.
master gtk2: The extension is not needed and the xpm is installed in pixmaps
branch gtk3: xpm is NOT installed in pixmaps

Comparing how both projects are setup in the makefiles, bibledit-desktop/pix/Makefile.am in gtk2 has:

appicondir = $(datadir)/pixmaps
appicon_DATA = bibledit-desktop.xpm

adding the lines back into gtk3 makefile now copies the xpm to pixmaps and the icon is rendered in menu.

I can make PR's if no one disagrees with this assessment.

@postiffm
Copy link
Owner

Sounds good. Thanks @Pickle

postiffm added a commit that referenced this pull request Aug 29, 2019
    This includes the *.desktop file rename and corrections.

    Closes #115

    MAP added 8/24/2019: Resolve small conflict introduced by PR #124
postiffm added a commit that referenced this pull request May 11, 2020
    This includes the *.desktop file rename and corrections.

    Closes #115

    MAP added 8/24/2019: Resolve small conflict introduced by PR #124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants