This repository has been archived by the owner. It is now read-only.

New terminal-notifier formula #18511

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants
Contributor

fish2000 commented Mar 15, 2013

The terminal-notifier utility (analogous to Growl's growlnotify tool) offers a CLI for displaying Apple Notification Center messages.

To use the program as distributed, one must call the inner binary of an app bundle (due to a library-linking quirk the author notes, in https://github.com/alloy/terminal-notifier/blob/master/README.markdown) – this formula sidesteps this awkward mode of execution with an exec script (see https://github.com/fish2000/homebrew/commit/e5e81985cefea17bfb076f6d00a9fe4a40e525b8#L0R14).

If there is interest in this program, I can add a devel or HEAD option to allow the terminal-notifier XCode project to be built from its Git source, via xcrun.

Contributor

fish2000 commented Mar 15, 2013

@adamv gotcha, thanks – sorry about these issues. They're easy to fix, I'll send a new pull request (or add commits here, if that is preferable).

Contributor

adamv commented Mar 15, 2013

If this pull request was made from a non-master branch, you can rebase and squash to a single commit and update this pull request in place with git push -f

Contributor

fish2000 commented Mar 15, 2013

@adamv I just re-pushed the commit with the changes squashed in.

Thanks for the tip – updating a pull request in place is a neat trick.

Contributor

fish2000 commented Mar 15, 2013

@adamv The pull request now has the test code in its proper place.

N.B. I originally drafted the formula using the test do … end syntax – my impetus, in this case, for eschewing the best-practice was this: since the this tool’s purpose is the display of textual feedback via an ephemeral GUI, the opportunity to use test code’s display of an exemplary transient notification would provide an role analogous to the Formula object’s caveat method, through like a show-and-don’t-tell UX tactic, if you will. I had also assumed that test blocks were to contain automation-friendly unit-test-style code (a mistake, I now realize). I’m just sayin – in retrospect, it was totally a bunch of overthinking on my part.

@jacknagel jacknagel commented on an outdated diff Mar 16, 2013

Library/Formula/terminal-notifier.rb
+ option 'quiet', "Supress the test notification display"
+
+ def install
+ # Write an executable script to call the app bundles' inner binary
+ # See the developers' note on the matter in the project README:
+ # https://github.com/alloy/terminal-notifier/blob/master/README.markdown
+ prefix.install Dir['*']
+ inner_binary = "#{prefix}/terminal-notifier.app/Contents/MacOS/terminal-notifier"
+ bin.write_exec_script inner_binary
+ chmod 0755, Pathname.new(bin+"terminal-notifier")
+ end
+
+ test do
+ # Display a test notice
+ if not build.include? 'without-test' and \
+ not build.include? 'quiet'
@jacknagel

jacknagel Mar 16, 2013

Contributor

The test doesn't need an option to disable the test, and you can just use ARGV.verbose? to check for the -v flag rather than invent a new option here.

Contributor

fish2000 commented Mar 16, 2013

@jacknagel Indeed, that makes a lot of sense… I just pushed the update as per your comments.

@fish2000 @fish2000 fish2000 New terminal-notifier formula (first version, revision 3)
The `terminal-notifier` utility (analogous to Growl's `growlnotify` tool) offers a CLI for displaying Apple Notification Center messages.

To use the program as distributed, one must call the inner binary of an app bundle (due to a library-linking quirk the author notes, in https://github.com/alloy/terminal-notifier/blob/master/README.markdown) – this formula sidesteps this awkward mode of execution with an exec script (see L#15).

New terminal-notifier formula (with test code codoned off apropos its function, redundant calls removed, and adjusted syntax in respect of Homebrew code conventions)
e5e8198
Contributor

samueljohn commented Mar 16, 2013

Cool stuff. I use terminal-notifier already a lot. I even wrote a little wrapper function (zsh/bash) so I can note "title,optional" "some stuff I want to do" in the shell and feed todos and other little notes into notification center.

Contributor

adamv commented May 3, 2013

Ping.

Contributor

adamv commented May 22, 2013

Can we get these updates?

@samueljohn samueljohn closed this in 74abcd1 Jun 3, 2013

Contributor

samueljohn commented Jun 3, 2013

Thanks for this formule. Perfectly done. Extra points for the Test 👍
Works nicely!

Sorry for me being so slow on this one.

Contributor

petere commented Jun 4, 2013

You can install this using gem. Why does it need to be in Homebrew?

Owner

MikeMcQuaid commented Jun 4, 2013

You can also install gist and hub using gem. This has nothing to do with Ruby and has no Ruby dependencies (and is useful at a wider level) so it makes sense to have a formula.

Contributor

samueljohn commented Jun 4, 2013

I did some more work here. It seems to build from source now and I added OS X 10.8 as a requirement (because of notification center). However, I had to deactivate the code signing step (because, obviously we don't have a cert). I guess, this disables part of the functionality. Though I am not 100% sure.
terminal-notifier -message 'brew test' -open 'http://brew.sh' does no call back and does not open the url when I click on the message. That worked with my terminal-notifier that I downloaded directly (or via the gem).

So since this is not pure Ruby code, it would make sense to have it in homebrew. But the certificate issue starts some doubts in me. Not sure. Perhaps I should ask upstream to get clarification.

Contributor

samueljohn commented Jun 4, 2013

reopening because of certificate issue and further discussion whether to homebrew or not to brew.

@samueljohn samueljohn reopened this Jun 4, 2013

Owner

MikeMcQuaid commented Jun 4, 2013

It's in Homebrew now so I don't think it makes sense to revoke it. It seems to compile (if not run) on Lion just not Snow Leopard. Let's try and have a test that works OK on the bot (or detects it somehow and does a different test depending on location).

Contributor

mistydemeo commented Jun 4, 2013

Codesigning shouldn't be necessary to run on Mt. Lion, just to distribute. I don't think it's a problem.

Contributor

samueljohn commented Jun 4, 2013

not for distribution but perhaps to make Notification Center open the given command or url. Not sure though.

Contributor

samueljohn commented Jun 4, 2013

I agree with you @MikeMcQuaid. I'll see if I can come up with a test.
I use this software personally, so I have some intentions, to make it a good formula :-)

Contributor

samueljohn commented Jun 4, 2013

perhaps for discussion: norio-nomura/usernotification#2
I asked someone with greater knowledge about notification center ...

Contributor

samueljohn commented Jun 6, 2013

Turns out, I was too stupid and the terminal-notifier works without code signing. I just had to reboot my Mac for whatever reasons (perhaps because I had terminal-notifier before). I will remove the caveat.

@samueljohn samueljohn closed this Jun 6, 2013

@handyman5 handyman5 pushed a commit to handyman5/homebrew that referenced this pull request Oct 7, 2013

@fish2000 @samueljohn fish2000 + samueljohn New terminal-notifier formula (first version, revision 3)
The `terminal-notifier` utility (analogous to Growl's `growlnotify` tool) offers a CLI for displaying Apple Notification Center messages.

To use the program as distributed, one must call the inner binary of an app bundle (due to a library-linking quirk the author notes, in https://github.com/alloy/terminal-notifier/blob/master/README.markdown) – this formula sidesteps this awkward mode of execution with an exec script (see L#15).

New terminal-notifier formula (with test code codoned off apropos its function, redundant calls removed, and adjusted syntax in respect of Homebrew code conventions)

Closes #18511.

Signed-off-by: Samuel John <github@SamuelJohn.de>
4d0ab60

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.