-
Notifications
You must be signed in to change notification settings - Fork 339
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
gtk3ui and macos fixes #273
base: develop
Are you sure you want to change the base?
Conversation
instead of hardcoded accelerators. This also fixes accelerator remapping for macOS in Deluge 2.x Also, use a better syntax for Gdk and Gtk imports. Nothing is gained from importing individual functions or classes (this is even explicitly discouraged for C/C++ programs) and it makes code harder to read.
Set the environment up properly and dump settings under ~/Library instead of ~/.config; while trying to import them from the previous location. This is more idiomatic for mac and also in theory could allow Deluge 1.3.15 and 2.0.x to coexist.
Heey Lord-Kamina thanks for your work, hope the deluge team finds some time to look at this soon. I'm using the .app you provided atm and was wondering if you could also upload the newer build with the deluge-bin fix. |
Will get to it sometime this weekend. The new build I upload will support handling of magnets (only on macOS 10.13+ though) |
This is dependent on a patch to gtk-mac-integration/GTKOSXApplication (MR already sent upstream)
@Boelensman1 sorry, I got absorbed doing other shit on the weekend. Here you go, https://www.dropbox.com/s/c51t7y5kh7za2kl/Deluge.app.7z?dl=1 Note, if you're on macOS 10.13+, this should open magnets natively, without relying on the Magnet Handler app. (That functionality cannot be replicated for now though because it depends on an as-yet unmerged PR over at the gtk-mac-integration repo) |
Thanks so much @Lord-Kamina! It works brilliantly |
@Lord-Kamina thank you for the updated code, quick question, how can I build it myself on my computer? Is there a documentation I can refer to? |
If you're using macports, you should install all dependencies and then run the |
@Lord-Kamina thanks, FYI, it doesn't build on Big Sur, some library lookup failing during the setup.py. Commenting line 88 and 89 of MacOGraph.py resolves the above issue #if not os.path.exists(pathname):
# raise ValueError("%r does not exist" % (pathname,)) |
I've recently updated to Big Sur myself. I haven't tried building it (I have to rebuild dependencies and my dev environment probably) but it's still running at least. Will take a crack at rebasing master and seeing what the issue is with the build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lord-Kamina I did cherry-pick some changes and left comments on a few other items
@@ -534,14 +534,15 @@ def run(self): | |||
] | |||
_package_data['deluge.ui.gtk3'] = ['glade/*.ui'] | |||
|
|||
setup_requires = ['setuptools', 'wheel'] | |||
setup_requires = ['setuptools', 'wheel', 'py2app'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
py2app
should not be requirement as that is macos specific
install_requires = [ | ||
'twisted[tls]>=17.1', | ||
# Add pyasn1 for setuptools workaround: | ||
# https://github.com/pypa/setuptools/issues/1510 | ||
'pyasn1', | ||
'rencode', | ||
'pyopenssl', | ||
'pygobject', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a requirement for minimal running since this does not apply to deluged
.
This does raise the question that do potentially other ui-specific requirements need scrutiny e.g. mako
It could however be included under extra_requires if needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not terribly familiar with python setuptools. What would be the preferred way to have this "just work®", adding it under extra_requires as you say? I worked on this quite a long time ago, but if I added this it's most likely because I got an error without it along the way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, mako
is included into install_requires
but is WebUI-specific only.
<signal name="activate" handler="on_menuitem_preferences_activate" swapped="no"/> | ||
<accelerator key="P" signal="activate" modifiers="GDK_CONTROL_MASK"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if these accelerator keys were not removed from the glade files and instead we found an alternative way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's possible to implement portable shortcuts without removing them from the glade files.
Hey, know this is an old issue. Is there a possibility of the app being updated to work on macOS Sonoma? There seems to be an issue with the python executable & the python path not being set after updating. Thanks in advance |
Hi @Lord-Kamina, Do you think is it possible to do an update for Sonoma ? Thank you 😊 |
id compile it myself but I'm running into this issue. Traceback (most recent call last): |
This fixes some standing issues with accelerators on macOS, as well as makes it easier to produce a working app bundle using macports (I made my changes in a new script and bundle-file, in case people prefer jhbuild still; it shouldn't be too hard to mimic what I did for macports to be used with jhbuild)
Of note, it appears in the future it would be desirable to drop GtkosxApplication altogether and use Gtk.Application instead but I'm not sure I am familiar enough with either python nor GTK3 to make those changes myself.
P.S. You can take my .app for a spin here: https://www.dropbox.com/s/zy0y4u41bvp6huk/Deluge.app.zip?dl=1