Closed martinlindhe closed 6 years ago
Hey, sure, why not join force.
I like https://github.com/go-toast/toast package, it seems it generates powershell script and then exec that from tmp, beeep currently just use msg
command, that needs to be fixed. Maybe also powershell can run command from string, so it doesn't need to write file just for notification, we can just use some bits from that package, i.e. copy some of it.
https://github.com/deckarep/gosx-notifier , I don't like approach with 2MB binary included in code, my solution just uses osascript
that should be by default in macOS (not sure from which version). Notify() function in beeep currently doesn't have option for image, osascript will use default icon from app bundle.
As for alerts beeep on Linux behaves same as beep
program that is included in every distro, on Windows it uses Beep() function with syscall, and on macOS again osascript is used to send bell
. I don't like hard coded paths, for example I don't have paplay
and freedesktop sounds installed.
I think pulseaudio can be configured to handle X11 bell events, so just sending bell character can be enough.
Great, I'll get back to you
https://github.com/deckarep/gosx-notifier , I don't like approach with 2MB binary included in code, my solution just uses osascript
I agree I also don't like the 2MB binary thing. I remember playing around with osascript too, but no longer remember why I didn't go with that :-)
I don't like hard coded paths, for example I don't have paplay and freedesktop sounds installed.
Also fair. I didn't put much effort in the Linux wrapper. Your Beep solution seems more generic and robust.
So, what do you say about the following, i can create a PR for each bullet point if you want.
windows 10 proper notifications using toast.
Alert() function, like a notify but uses system alert sound too. (I think the Linux version of this should use your code for notification + beep sound to not rely on installed commands).
extend Notify() to allow for specifying title and icon to use (this would break your API. not sure that is desired. also maybe not super important. and not all operating systems will allow showing app icon at the moment)
Everything looks good, I also wanted to add icon so breaking API is OK for now.
I guess toast will work on Windows 10 only, what about ver 7, can we detect it and fallback to something, maybe just to msg
like it is currently?
On Linux icon can be name in a freedesktop.org-compliant icon theme or file:// URI path, on Windows I guess just path, on macOS osascript will use icon from app bundle, and on Web that can be path or even http:// URI. Maybe that can be noted in docs comments.
Also, for Beep() maybe add to docs how to configure pulseaudio for bell events, something like this https://wiki.archlinux.org/index.php/PulseAudio#X11_Bell_Events . What do you think?
Everything looks good, I also wanted to add icon so breaking API is OK for now.
Okay, great!
I guess toast will work on Windows 10 only, what about ver 7, can we detect it and fallback to something, maybe just to msg like it is currently?
Yes, it requires Windows 10. Fallback to your existing cmd solution sounds good to me.
Also, for Beep() maybe add to docs how to configure pulseaudio for bell events, something like this https://wiki.archlinux.org/index.php/PulseAudio#X11_Bell_Events . What do you think?
Is this required today for https://github.com/gen2brain/beeep/blob/master/beep_linux.go ? I haven't yet tested your code under Linux, but was hoping it would work out of the box.
It is not required, if it can not open tty0 or evdev input device (both need pcspkr
module loaded) it will output bell character, with xset b on
configured it should beep, but if pulseaudio is configured like above instead of beep it will play the user configured .ogg file.
Btw. on Linux Beep() in some loop can also play tunes like from this collection https://github.com/ShaneMcC/beeps .
Holding off with extend Notify() to allow for specifying title and icon to use
until #3 has been merged
Great, #3 is merged.
In Windows maybe it is easier to get version with https://godoc.org/golang.org/x/sys/windows#GetVersion , then check major/minor like this fmt.Printf("Windows version %d.%d (Build %d)\n", byte(v), uint8(v>>8), uint16(v>>16))
.
Or even better, with standard lib only, something like this:
import (
"syscall"
)
var (
kernel32 = syscall.NewLazyDLL("kernel32.dll")
getVersion = kernel32.NewProc("GetVersion")
)
func getVersion() uint32 {
ret, _, _ := getVersion.Call()
return uint32(ret)
}
Will have to check what this returns on Windows 10 and 7, in vmware.
I tried the GetVersion syscall on Windows 10.
While "cmd ver" returns Microsoft Windows [Version 10.0.16299.125]
and is thus easy to parse,
The below go code returns version 6.2 build 9200
package main
import (
"fmt"
"syscall"
)
var (
kernel32 = syscall.NewLazyDLL("kernel32.dll")
getVersionProc = kernel32.NewProc("GetVersion")
)
func main() {
major, minor, build := getWindowsVersion()
fmt.Printf("version %d.%d build %d", major, minor, build)
}
func getWindowsVersion() (uint, uint, uint) {
ret, _, _ := getVersionProc.Call()
// on "Windows 10, 10.0.16299.125", ret is 0x23f00206 = 0b100011111100000000001000000110
val := uint(ret)
verPart := val & 0xFFFF
minor := (verPart >> 8) & 0xFF
major := verPart & 0xFF
build := (val >> 16) & 0xFFFF
return major, minor, build
}
I just found this "For applications that have been manifested for Windows 8.1 or Windows 10. Applications not manifested for Windows 8.1 or Windows 10 will return the Windows 8 OS version value (6.2). "
Not a big deal, I guess cmd ver
is fine then.
Ok, I added alert_js.go , and I had to add build tags for that, gopherjs js tag is not official. You can test it here http://81.4.106.254/a/alert.html .
Really cool!
I've updated my repo at https://github.com/martinlindhe/notify with a deprecation note, linking here instead.
Hey, just saw your library
I made this a while ago, https://github.com/martinlindhe/notify and I am wondering if you would like to join forces?
I haven't looked deep, but our use case and implementations seems similar.