jakemcc / test-refresh

Refreshes and reruns clojure.tests in your project.
393 stars 28 forks source link

Can't use user specified notification command #23

Closed hugocorbucci closed 9 years ago

hugocorbucci commented 9 years ago

I'm trying to use the notify-command option but, due to the way terminal-notifier (and many other command line tools) work, having the last argument being a series of words separated by spaces causes the command to miss most of it. That is terminal-notifier -message Passed All Tests only shows "Passed" while terminal-notifier "Passed All Tests" shows the full message.

I had to create a small bash file to notify with this format:

#!/bin/bash
terminal-notifier -title "Engine Server Tests" -message "$*"

Maybe the message could be wrapped in double quotes? Or be in a variable that can be used as preferred in the notification command?

jakemcc commented 9 years ago

Hmm, that is interesting. I'm not able to reproduce the same problem locally.

My :test-refresh section of my profiles.clj looks like :test-refresh {:notify-command ["terminal-notifier" "-title" "Tests" "-message"]} and see the full "Passed all tests" message in the notification command.

How do you have your project.clj or profiles.clj setup? What version of terminal-notifier?

hugocorbucci commented 9 years ago

terminal-notifier (1.6.2)

Ah! I think I figured it out. I has previously just :test-refresh {:notify-command ["terminal-notifier" "-message "]}. In that case, nothing shows in the messages. That extra space after the message causes it to show no text in the notification.

Maybe it's just a matter of detailing the documentation with that example. Or probably even better, strip spaces from beginning and end of arguments? Or both? :)

jakemcc commented 9 years ago

Did a bit of work on the README. Let me know what you think. It calls out the example a bit more.

Its probably safe to trim the inputs. Might think about it some first.

hugocorbucci commented 9 years ago

I think the example will pretty much make it obvious for others on the usage. I can still imagine some command line apps where it would be hard to have it as a last argument but, until the problem occurs to anyone, I'd ignore.

Trim the inputs is just a matter of avoiding no working inexplicably for silly typos. Doesn't seem too hard to do (I'm still learning clojure so I don't offer a patch yet. Maybe in a couple of days).

Thanks!