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

Add ability to integrate the client with desktop environment on Linux #38

Closed
wants to merge 3 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jul 20, 2020

On Linux a new menu item "Settings" --> "Integrate with desktop environment" is available now (non-regtest only).

Here are some screenshots:

Screenshot from 2020-07-20 18-36-52

DeepinScreenshot_select-area_20200720190707

Screenshot from 2020-07-20 18-41-08

Closes #31


Localization of the *.desktop files is out of this PR scope.

@jonatack
Copy link
Member

Concept ACK

@hebasto
Copy link
Member Author

hebasto commented Jul 20, 2020

Updated 2bd1394 -> 5dc09b5 (pr38.01 -> pr38.02, diff):

  • improved user-faced "Comment" field
  • screenshot in the OP has been updated

@luke-jr
Copy link
Member

luke-jr commented Jul 20, 2020

Approach NACK. Just restore the .desktop file removed in fa0e1e2 and add the missing bits to configure.ac. There's no reason to go through the application to generate this...

@caiofaustino
Copy link

Tested, works well for me.
File created at ~/.local/share/applications/org.bitcoincore.BitcoinQt.desktop
Screenshot from 2020-07-21 00-44-20

@caiofaustino
Copy link

caiofaustino commented Jul 20, 2020

Approach NACK. Just restore the .desktop file removed in fa0e1e2 and add the missing bits to configure.ac. There's no reason to go through the application to generate this...

Can you explain what behaviour that approach would have for a user downloading the binary from the website?
I'm new to most of this, my understanding was that configure.ac. only affected compilation.

@luke-jr
Copy link
Member

luke-jr commented Jul 21, 2020

Can you explain what behaviour that approach would have for a user downloading the binary from the website?

There'd presumably be a .desktop file in the tarball they could install to the right place.

@caiofaustino
Copy link

There'd presumably be a .desktop file in the tarball they could install to the right place.

Thanks for the explanation, yeah that could be an alternative, although I think this one is more user friendly, I'm not sure what level of technical expertise is expected from the users.

@hebasto
Copy link
Member Author

hebasto commented Jul 29, 2020

Approach NACK. Just restore the .desktop file removed in fa0e1e2 and add the missing bits to configure.ac. There's no reason to go through the application to generate this...

Can you explain what behaviour that approach would have for a user downloading the binary from the website?

There'd presumably be a .desktop file in the tarball they could install to the right place.

Placing a *.desktop file into the tarball requires placing icon files into it as well.

Manually installing a *.desktop and icon files seems not user-friendly.

As a *.desktop and icon files usually are parts of packages (e.g., https://snapcraft.io/bitcoin-core, https://packages.debian.org/sid/main/bitcoin-qt), it is possible that a configure option is required to disable this feature for package maintainers.
@MarcoFalke What do you think about it?

@luke-jr
Copy link
Member

luke-jr commented Jul 29, 2020

Packagers would simply switch to the bundled icon/desktop files...

@hebasto
Copy link
Member Author

hebasto commented Jul 29, 2020

Packagers would simply switch to the bundled icon/desktop files...

Yes, that is what I mean: if icon/desktop files are bundled into the package there is no need to compile the feature introduced in this PR; therefore it could be disabled by the configure script.

@hebasto
Copy link
Member Author

hebasto commented Aug 4, 2020

Approach NACK. Just restore the .desktop file removed in fa0e1e2 and add the missing bits to configure.ac. There's no reason to go through the application to generate this...

The alternative approach is bitcoin/bitcoin#15068

@hebasto
Copy link
Member Author

hebasto commented Aug 26, 2020

Rebased 5dc09b5 -> 95c0cff (pr38.02 -> pr38.03) due to the conflict with bitcoin/bitcoin#15704.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK on Arch Linux and KDE Plasma. Works smoothly.
Also tested to make sure this option doesn't show up on macOS.

One question: the option does not show up when testing on FreeBSD, how do we add support for the BSD's?

@hebasto
Copy link
Member Author

hebasto commented Dec 4, 2020

@jarolrod

Tested ACK on Arch Linux and KDE Plasma. Works smoothly.
Also tested to make sure this option doesn't show up on macOS.

Thank you for testing.

One question: the option does not show up when testing on FreeBSD, how do we add support for the BSD's?

I'm not familiar with BSD world, except of macOS :)

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@hebasto hebasto marked this pull request as draft December 30, 2020 19:55
@hebasto
Copy link
Member Author

hebasto commented Apr 30, 2021

Closing and leaving it up for grabs.

@caiofaustino
Copy link

What does leaving for grabs mean? Is this feature still considered or does it need further discussion?

@hebasto
Copy link
Member Author

hebasto commented May 1, 2021

What does leaving for grabs mean? Is this feature still considered or does it need further discussion?

Anyone could continue this work. Although, a new pr will be a subject for discussion.

@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate .desktop file for Gnome and other desktop environments.
6 participants