guard / notiffany

wrapper for popular notification libs
MIT License
34 stars 25 forks source link

:type appears to be a symbol, not a string #18

Open mikelococo opened 8 years ago

mikelococo commented 8 years ago

Type is documented in several places as being a string:

And sometimes referenced as a string: https://github.com/guard/notiffany/blob/0618490eb5332d1e6162215f508674a796b3e2f5/lib/notiffany/notifier/libnotify.rb#L47

Whenver I snoop on the values of type, they look like a symbol though. You may wish to update the docs and tweak that libnotify priority switch if they're intended to always be symbols.

e2 commented 8 years ago

I'm not that much of a fan of documentation, really. And I don't use notification much, either.

(I'd actually remove the internal documentation, unless it documented something really surprising).

I guess it's intended to be a symbol:

https://github.com/guard/notiffany/blob/0618490eb5332d1e6162215f508674a796b3e2f5/lib/notiffany/notifier/base.rb#L103

No one complained yet really, so if you can create a PR, I'll gladly pull it in.

I think the libnotify should be fixed to use fail, so the priority can be shown properly. I think of it as an enhancement, so no compatibility problems here.