Open daniel-abramov opened 7 years ago
Looks like this died on CI due to a timeout on the LLVM server. I'm kicking it to run again. Assuming it builds, is this ready to land, or should I just bring in the dependency update commit?
Ah, I forgot that it automatically updates the pull request, just made some local changes here :)
Well, basically if you want to know the current state from my side: I've been using your crate with a patched mac version (the one we discussed in a separate issue dedicated to Mac OS X support) for a while and it works fine for Mac, but currently it lacks the possibility to add additional menu items to the tray icon in Mac. But since my branch diverged from master much, I decided to rebase it, bringing this "basic Mac OS X support" and fixing some Linux issues which I observed when I tried to compile it in Linux.
Unfortunately I did not have time to write the full support for Mac so far, but I'll try if I find some time in future (unless someone else implements this), I already know how to do it.
Apart from that there are also several things which I think have to be changed in this crate to make it usable for production, namely:
Application
has to be accessible from different threads (there are several ways it can be achieved, the good way would be to implement it similar Handle
and Remote
work for tokio framework in Rust), so that the other thread can schedule a quit()
. Otherwise it's not possible to close the tray icon when other thread panics and when the developer wants to "gracefully close the application". I think it was discussed already in one or another issue. Currently I have to do sys.exit()
to close the app in this case which is of course not the best thing to do.Yeah, this crate started with good intentions, but I got sidetracked onto some C# and typescript stuff that has kinda eaten my life. I'm /still/ hoping to get more work done on this, and luckily some other project work I need to do involves learning tokio, so I can check out those patterns there to get threaded access working. Also, yay, had no idea winapi 0.3.x was out yet. :D
@qdot here is by the way another important issue which I believe I mentioned somewhere in other issues in the past -- the tray icon library in the way it is written now will not work for every platform.
I tried to adapt my branch to the current state from your master branch and I've noticed that after I did that, my OS X application crashes when I try to use the library, after checking the code I've found a reason for that.
Basically we cannot create an "event loop" inside the tray icon library in an arbitrary thread as there are some operating systems (OS X) where we have to stick to the main thread only. Because of that I'm not sure if we can make this library truely crossplatform in the current architecture, because even if we update the architecture and documentation, pointing out that the Application
has to be created in the main thread. That's probably one of the reasons why Qt's main application is also bound to the main thread and also there are certain limitations in other libraries (https://github.com/glfw/glfw/issues/136).
Currently the user of the library has to create an Application
object and then call wait_for_event()
at the end. Here are many questions which are not easy to solve:
Application::new()
should be called only from the main thread (due to the above mentioned limitations in some operating systems).wait_for_message()
does"? On some operating systems we may start a message loop in an arbitrary therad (Windows), in others we have to call NSApplication::run
(OS X), and in case of OS X we have to call it from the main thread. But in this case it's not clear when wait_for_message()
returns. Is it returned when the icon is killed? Or when the event loop is stopped? It would be logically to return when the icon is killed, but in case if we called NSApplication::run
(OS X) before, then it would mean that we have to terminate the NSApplication
(which may be an unexpected action for some people).In other words, there are a plenty of things we have to think about. Perhaps the best thing which can be done is to write a library which provides something similar to libappindicator
, when icon specific functionality is provided, but the reactor (event loop) is a choice of the user. But that would lift only some of the limitations though...
Could you please merge this?
Maybe the code isn't perfect, but it's a great start. Will be easier to contribute if this is merged and it has just been lying here for over a year now.
@application-developer-DA I just rebased the snapview/systray-rs MacOS patches over here in an effort to try to get macOS code working, they're on the "macos" branch. I had to add a few more implementations (and I would like to speak to me from 4 years ago about why Window wasn't implemented as a trait...) to get things compiling, but I'm getting a segfault when running the example.
2020-02-15 16:07:07.051 trayicon[89980:43951120] -[NSApplication terminate]: unrecognized selector sent to instance 0x7fa7745054f0
2020-02-15 16:07:07.051 trayicon[89980:43951120] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSApplication terminate]: unrecognized selector sent to instance 0x7fa7745054f0'
*** First throw call stack:
(
0 CoreFoundation 0x00007fff32e1fa7d __exceptionPreprocess + 256
1 libobjc.A.dylib 0x00007fff5d4f4a17 objc_exception_throw + 48
2 CoreFoundation 0x00007fff32e99886 -[NSObject(NSObject) __retain_OA] + 0
3 CoreFoundation 0x00007fff32dc18ef ___forwarding___ + 1485
4 CoreFoundation 0x00007fff32dc1298 _CF_forwarding_prep_0 + 120
5 trayicon 0x0000000103fe9aea _ZN60_$LT$$LP$$RP$$u20$as$u20$objc..message..MessageArguments$GT$6invoke17h69e32fa6f096a5f2E + 58
6 trayicon 0x0000000103fe963e _ZN4objc7message8platform15send_unverified17h042675139b5a0ef4E + 110
7 trayicon 0x0000000103fd3071 _ZN7systray3api3api6Window4quit17he4923e26cbc4226bE + 289
8 trayicon 0x0000000103fd6108 _ZN7systray11Application4quit17hd7f0d033b3b0d357E + 24
9 trayicon 0x0000000103fd6182 _ZN7systray11Application16wait_for_message17h09c979c55e9dcbe7E + 114
10 trayicon 0x0000000103fc99bf _ZN8trayicon4main17h62f69ed1e89febe7E + 351
11 trayicon 0x0000000103fc9b12 _ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17hd8f76e20a97191b6E + 18
12 trayicon 0x0000000103ff5748 _ZN3std9panicking3try7do_call17ha32f9b92275332dfE + 24
13 trayicon 0x0000000103ff733b __rust_maybe_catch_panic + 27
14 trayicon 0x0000000103ff603e _ZN3std2rt19lang_start_internal17he40e6c5bb3d144c3E + 542
15 trayicon 0x0000000103fc9af2 _ZN3std2rt10lang_start17hcbf8acef059cc370E + 66
16 trayicon 0x0000000103fc9aa2 main + 34
17 libdyld.dylib 0x00007fff5ecc23d5 start + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException
zsh: abort cargo run --example trayicon
I haven't had a chance to dig into it too much yet, but thanks to working on https://github.com/deviceplug/btleplug and writing the CoreBluetooth impl for that, I've got some ObjC/Rust experience now, so I'll try to help out where I can, if/when I have time.
@qdot oh, I did not expect you would return working on this ;)
I'll try to check in the next days which branch do I use in some of the projects which currently rely on a tray icon crate (they work fine on Catalina and don't crash). I feel that a tray icon crate may require some refactoring, but I just was not able to do that as the priorities changed and now the tray icon part is a very small part of the product I was working on back then.
On Linux we rely on libappindicator which seems to be a bit outdated as it does not seem to compile on the latest Rust version anymore (due to some old dependencies).
@qdot oh, I did not expect you would return working on this ;)
Yeah I sometimes end up returning to projects. :)
I'll try to check in the next days which branch do I use in some of the projects which currently rely on a tray icon crate (they work fine on Catalina and don't crash). I feel that a tray icon crate may require some refactoring, but I just was not able to do that as the priorities changed and now the tray icon part is a very small part of the product I was working on back then.
Ok, don't worry about it too much if you're not using it much anymore. I just saw you had an update a couple of months ago so I didn't know if things were at least somewhat working.
On Linux we rely on libappindicator which seems to be a bit outdated as it does not seem to compile on the latest Rust version anymore (due to some old dependencies).
Yeah, I updated all of the libappindicator dependencies, those changes came in with systray 0.4.0 which I released yesterday.
I just saw you had an update a couple of months ago so I didn't know if things were at least somewhat working.
Hm... I think last time I commited something to that repository was a year ago. Oh, by the way I checked one of the solutions where we use the tray icon and it turned out that we don't have any menus there, just a tray icon, that's probably the reason it does not crash. Maybe the example from master
is indeed broken.
From what I can see, the only thing we do with the systray
crate in our case is:
let buffer: &[u8] = include_bytes!("../resources/icon.ico");
let mut app = systray::Application::new()
.expect("Bug in the tray icon library, could not create tray icon");
if app.window.set_icon_from_buffer(buffer, 48, 48).is_ok() {
info!("Tray icon app has been sucessfully created");
app.window.wait_for_message();
} else {
error!("We could not set the icon for the tray icon, so no tray icon will be used");
}
UPD. Oh, wait, the tray-icon example for Mac is simple and should work actually. I've just tried to pull the latest master
branch from the fork and run the example, it runs without segfaults.
Partially implements #4