cratonica / trayhost

Cross-platform Go library to place an icon in the host operating system's taskbar
MIT License
283 stars 59 forks source link

Merge with the forks #3

Open bbigras- opened 10 years ago

bbigras- commented 10 years ago

This repo has two forks with a couple of commits each. Maybe it would be nice to merge them.

dmitshur commented 10 years ago

:+1: to the idea, it would certainly be very nice. But I suspect it will be a bit of work to unify the divergences, and not everyone may be happy with other's design decisions (but there will be a lot of common stuff that we will agree on).

I've made quite a few additions/improvements/simplifications (primarily for OS X) on my https://github.com/shurcooL/trayhost fork, and it is used by an app.

marbemac commented 9 years ago

@shurcooL Love the additions! I couldn't get notifications to work (OS X 10.9.5). Nothing happens, no errors. https://github.com/deckarep/gosx-notifier seems to work fine for the moment - but it would be great if I could figure out why the trayhost notifications are not working, and consolidate to just the trayhost package.

dmitshur commented 9 years ago

@marbemac Thanks. I know why they're not working and I can guide you to make it work. In fact, I should document that, as it's a useful thing that can save other people time (it took me hours of searching through apple docs and trying things to figure out what was wrong). Also add better error handling.

I will do that tonight and ping you (if you can't wait and want a hint now: the short version is the executable must be inside a proper .app bundle complete with Info.plist file that has CFBundleIdentifier set - notification center won't be available otherwise).

marbemac commented 9 years ago

@shurcooL A little writeup would be fantastic. No rush, this is an app in development and I have plenty of other things to finish.

marbemac commented 9 years ago

@shurcooL Was also wondering if you had played around with manipulating the dropdown menu after EnterLoop() has been called? For example, in my menu I list the projects the user has created in-app. When they create or delete a project, ideally I'd add/remove it from the menu.

dmitshur commented 9 years ago

I have not done that specifically, but it's quite a normal thing to want to be able to do. I imagine it should be doable.

I have added support to allow dynamically enabling/disabling already added menu items, which is somewhat related.

bbigras commented 9 years ago

Was also wondering if you had played around with manipulating the dropdown menu after EnterLoop() has been called?

@overlordtm's fork seems to have something like that. See 71b5f3f1e7189a85dbbb1d31e66d92dd2d1582e6. I'm also using his fork to be able to change the icon.

marbemac commented 9 years ago

@shurcooL Does the OS X terminal pop up when you run your app binary? I see the windows command prompt note in the trayhost readme, but do you know if there's a way to to stop the terminal in osx from opening?

dmitshur commented 9 years ago

It does not, because my app is a part of an .app bundle.

That is a requirement for Notification Center to work. It's also good because it can have an icon, contain resources inside, and is a normal way to have an app in OS X.

dmitshur commented 9 years ago
Applications $ tree ./Instant\ Share.app/
./Instant\ Share.app/
└── Contents
    ├── Info.plist
    ├── MacOS
    │   └── main
    └── Resources
        ├── Icon.icns
        └── icon@2x.png

main is the Go binary.

marbemac commented 9 years ago

That makes so much sense, I'll give it a go. Loving gotools.org btw, thanks for putting it up.

dmitshur commented 9 years ago

Let me know how it works.

Here's a very minimal (only the entries that are required, nothing extra) Info.plist file as reference:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>CFBundleIconFile</key>
    <string>Icon</string>
    <key>CFBundleExecutable</key>
    <string>main</string>
    <key>CFBundleIdentifier</key>
    <string>YourAppName</string>
    <key>NSHighResolutionCapable</key>
    <true/>
    <key>LSUIElement</key>
    <string>1</string>
</dict>
</plist>

CFBundleIdentifier needs to be set to something for Notification Center to work. LSUIElement is needed to make the app not appear in Cmd+Tab list and the dock while still being able to show a tooltip in the menu bar. NSHighResolutionCapable to enable Retina mode.

And thanks, glad you like gotools! I'm looking forward to improving it.

cratonica commented 9 years ago

I'm willing to accept pull requests, but some of the forks have extensive modifications and I'm not sure if they are API compatible. If there is a fork that better suites your needs, then, by all means, use it! Github has served its purpose.

dmitshur commented 9 years ago

I think it would be better for the Go community if we could manage to unify the forks somewhat. For example, my fork has some useful general trayhost-related additions, but they're only implemented for OS X because that's what I use. If someone else uses Windows and implements it there, that'd be great.

But like I said, doing so is hard and requires time, cooperation and making compromises. But as things are, if someone wants to use this library, they have more work cut out for them trying to pick a fork that works best for their needs, etc.

dmitshur commented 9 years ago

I think being (the only person with push rights) willing to accept pull requests is not enough; I think we'd need to get a couple of willing developers to become maintainers with push rights so there's more than 1 person who can accept PRs and make progress. And then we'd need to try to unify the API differences and merge existing efforts. It's hard work, and there may be disagreements, hard to know if we can resolve them or not. It's not easy. :)

overlordtm commented 9 years ago

Hi everyone!

Sorry for late response, but I was pretty much overwhelmed with work lately. I am also willing to contribute to effort of unifying different forks into one great package. But i think first we need to agree on common API. At least for me, my fork is used in production software, so I cannot sacrifice any fuctionality (but I can survive API change, as long as same things can still be done in different way), because everything is "must have" for software that uses this package. So far I am aware of 2 main forks (my and shurcooL's). My is cross platform, shurcooL's is AFAIK mor or less OSX only. I can say for myself that I prefer to programm for linux platform, so maybe we can divide work between multiple developers, each one covering his platform of expertise.

dmitshur commented 9 years ago

I have a suggestion for how to make this work. It will be a significant amount of work and require time from everyone involved, but I'm interested if others are too.

I suggest we create a new repo (perhaps under some organization) and start designing a common API from ground up, and use a pull request with full review from everyone involved to make decisions on the common API.

Once we have an API that satisfies everyone's needs, we can start contributing the missing implementation details for platforms. I can contribute the same features I've already implemented in my fork for OS X.

If all that goes successfully and the new joined effort becomes feature complete with my fork, I can remove my fork and switch to using the new common library in our app that uses my fork. We can then add the new library to awesome-go and announce it on go-nuts mailing list, and hopefully many people find it useful.

Advantages: listed above. Disadvantages: extra work/time compared to doing nothing.

cratonica commented 9 years ago

For now, I've added both overlordtm and shurcooL as collaborators for this project. Since this root project hasn't changed since you both forked, really this is a matter of you both resolving your changes. with each other to create the extension of this authoritative branch. This could be done in a separate branch in this repository which we can merge to master once we are done sorting things out.

If you'd prefer to start from scratch and form a new organization around this, then we can do that, but I personally don't think that's necessary.

dmitshur commented 9 years ago

Cool, that could work. Let's work in PRs and merge after getting a LGTM.

Here are the two APIs:

http://godoc.org/github.com/shurcooL/trayhost http://godoc.org/github.com/overlordtm/trayhost

The most important part is MenuItem. My fork has:

type MenuItem struct {
    Title   string
    Enabled func() bool // nil means always enabled.
    Handler func()
}

While @overlordtm's is:

type MenuItem struct {
    Title    string
    Disabled bool
    Handler  func()
}

And:

type MenuItemUpdate struct {
    ItemId int
    Item   MenuItem
}

Main difference I see is that I need to be able to enable/disable individual menu items when the menu appears. In OS X, there's a callback that happens just before the menu is displayed; it gives me a chance to run Enabled() and enable/disable the menu based on that. Can that be done on Windows/Linux?

@overlordtm, can you explain how MenuItemUpdate is used?

overlordtm commented 9 years ago

MenuItemUpdate is used to feed the update channel. It is actually just wrapper around MenuItem with added index (to know which item to update). Using func() for enabled should not be a problem. I will try to take a look at differences today after work (GMT+1 is my timezone)