[PATCH] make org-notify support for macOS desktop notification

[PATCH] make org-notify support for macOS desktop notification

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]

I found `org-notify` does not support macOS desktop notification. So I write a small patch for this.

Thanks for reviewing.


[-- Attachment #2: 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch --]
[-- Type: application/octet-stream, Size: 1655 bytes --]

From cfaa53d44a6bc5bcd62efcc77568f46a5ccf50b4 Mon Sep 17 00:00:00 2001
From: stardiviner <numbchild@gmail.com>
Date: Sun, 4 Jul 2021 08:13:43 +0800
Subject: [PATCH] org-clock.el: Make org-notify support macOS notification

* lisp/org-clock.el (org-show-notification): Add support for macOS
notification.
---
 lisp/org-clock.el | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index cd930e875..df66f2950 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -859,7 +859,7 @@ use libnotify if available, or fall back on a message."
 	    org-show-notification-timeout
 	    nil
 	    (lambda () (w32-notification-close id)))))
-	((fboundp 'notifications-notify)
+	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
 	 (notifications-notify
 	  :title "Org mode message"
 	  :body notification
@@ -870,6 +870,12 @@ use libnotify if available, or fall back on a message."
 	((executable-find "notify-send")
 	 (start-process "emacs-timer-notification" nil
 			"notify-send" notification))
+        ((and (eq system-type 'darwin) (fboundp 'osx-lib-notify2))
+         (osx-lib-notify2 "Org mode message" notification))
+        ((and (eq system-type 'darwin) (executable-find "osascript"))
+	 (start-process "emacs-timer-notification" nil
+			"osascript" "-e"
+                        (format "'display notification \"%s\" with title \"title\"'" notification "Org mode message")))
 	;; Maybe the handler will send a message, so only use message as
 	;; a fall back option
 	(t (message "%s" notification))))
-- 
2.30.1 (Apple Git-130)

Re: [PATCH] make org-notify support for macOS desktop notification

[Maxim Nikulin]

On 04/07/2021 07:23, stardiviner wrote:
> I found `org-notify` does not support macOS desktop notification. So I write a small patch for this.

I am surprised that there is no OS-agnostic function in Emacs that sends 
simple notification, suitable when no advanced feature are necessary. 
Only OS-dependent variants are implemented for Linux and Windows.

> +                        (format "'display notification \"%s\" with title \"title\"'" notification "Org mode message")))

Unsafe substitution of the argument. There is no guarantee that 
notification has no quote characters. I do not know, which link you 
would prefer:
- old https://xkcd.com/327/ "Robert'); DROP TABLE"
- recent 
https://arstechnica.com/gadgets/2021/06/mass-data-wipe-in-my-book-devices-prompts-warning-from-western-digital/ 
Wipe data from NAS (accordingly to some sources, device can be protected 
by firewall, it is enough to open in a browser a page with a malicious 
<img src="..."> element, e.g. in a comment of an earlier visitor)

The preferred way is to pass such parameters as separate arguments of 
`start-process'. I am not familiar with osascript, I hope, it does not 
additionally interpret strings passed to "display notification" to do 
something fancy things. Example with sh:

Current unsafe variant:

>    sh -c "`printf 'echo "%s: %s - %s"' 'some-command' '"; echo another action ; echo "' 'second arg'
With parameters passed as separate arguments to avoid interpretation of 
special characters:

>    sh -c 'echo "$0: $1 - $2"' 'some-command' '"; echo another action ; echo "' 'second arg'

Re: [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 125 bytes --]

I updated the patch, I found the package `osx-lib` contains solution. So I removed the directly osascript process invocation.

[-- Attachment #2: 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch --]
[-- Type: application/octet-stream, Size: 1384 bytes --]

From 9bfb5d0d983f083558ecba51368ac4980f6008c7 Mon Sep 17 00:00:00 2001
From: stardiviner <numbchild@gmail.com>
Date: Sun, 4 Jul 2021 08:13:43 +0800
Subject: [PATCH] org-clock.el: Make org-notify support macOS notification

* lisp/org-clock.el (org-show-notification): Add support for macOS
notification.
---
 lisp/org-clock.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index cd930e875..314233156 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -859,7 +859,7 @@ use libnotify if available, or fall back on a message."
 	    org-show-notification-timeout
 	    nil
 	    (lambda () (w32-notification-close id)))))
-	((fboundp 'notifications-notify)
+	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
 	 (notifications-notify
 	  :title "Org mode message"
 	  :body notification
@@ -870,6 +870,8 @@ use libnotify if available, or fall back on a message."
 	((executable-find "notify-send")
 	 (start-process "emacs-timer-notification" nil
 			"notify-send" notification))
+        ((and (eq system-type 'darwin) (fboundp 'osx-lib-notify2))
+         (osx-lib-notify2 "Org mode message" notification))
 	;; Maybe the handler will send a message, so only use message as
 	;; a fall back option
 	(t (message "%s" notification))))
-- 
2.30.1 (Apple Git-130)


[-- Attachment #3: Type: text/plain, Size: 1720 bytes --]




> On Jul 4, 2021, at 1:48 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
> 
> On 04/07/2021 07:23, stardiviner wrote:
>> I found `org-notify` does not support macOS desktop notification. So I write a small patch for this.
> 
> I am surprised that there is no OS-agnostic function in Emacs that sends simple notification, suitable when no advanced feature are necessary. Only OS-dependent variants are implemented for Linux and Windows.
> 
>> +                        (format "'display notification \"%s\" with title \"title\"'" notification "Org mode message")))
> 
> Unsafe substitution of the argument. There is no guarantee that notification has no quote characters. I do not know, which link you would prefer:
> - old https://xkcd.com/327/ "Robert'); DROP TABLE"
> - recent https://arstechnica.com/gadgets/2021/06/mass-data-wipe-in-my-book-devices-prompts-warning-from-western-digital/ Wipe data from NAS (accordingly to some sources, device can be protected by firewall, it is enough to open in a browser a page with a malicious <img src="..."> element, e.g. in a comment of an earlier visitor)
> 
> The preferred way is to pass such parameters as separate arguments of `start-process'. I am not familiar with osascript, I hope, it does not additionally interpret strings passed to "display notification" to do something fancy things. Example with sh:
> 
> Current unsafe variant:
> 
>>   sh -c "`printf 'echo "%s: %s - %s"' 'some-command' '"; echo another action ; echo "' 'second arg'
> With parameters passed as separate arguments to avoid interpretation of special characters:
> 
>>   sh -c 'echo "$0: $1 - $2"' 'some-command' '"; echo another action ; echo "' 'second arg'
> 
> 

Re: [PATCH] make org-notify support for macOS desktop notification

[Maxim Nikulin]

On 05/07/2021 10:50, stardiviner wrote:
> I updated the patch, I found the package `osx-lib` contains solution.
> So I removed the directly osascript process invocation.

I have no objections any more. On the other hand I have no access to 
macOS, so I have not tested this patch. Feel free to ignore comments 
from this message, they are mostly matter of taste.

I expect that a simple script "notify-send" may allow to avoid 
modification of code. Something like (untested, unsure concerning 
"quoted form of ...")

#!/usr/bin/env osascript
display notification (item 1 of argv)

However if osx-lib in is installed automatically, it may be more 
convenient. Unsure if some of currently supported linux distributions 
have notify-send that can not handle title as the first argument.

> -	((fboundp 'notifications-notify)
> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))

Does it mean that `notifications-notify' is bound but it does not work 
on macOS? If so, maybe it is better to put new clause for 'darwin above 
and to drop 'gnu/linux here. From my point of view, it is preferable to 
avoid additional requirement for `notifications-notify'. If someone will 
create a feature request for `notifications-notify' for macOS, it will 
just work without installing of additional packages as soon as such 
feature is implemented.

Re: [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
> 
> On 05/07/2021 10:50, stardiviner wrote:
>> I updated the patch, I found the package `osx-lib` contains solution.
>> So I removed the directly osascript process invocation.
> 
> I have no objections any more. On the other hand I have no access to macOS, so I have not tested this patch. Feel free to ignore comments from this message, they are mostly matter of taste.
> 
> I expect that a simple script "notify-send" may allow to avoid modification of code. Something like (untested, unsure concerning "quoted form of ...")
> 
> #!/usr/bin/env osascript
> display notification (item 1 of argv)
> 
> However if osx-lib in is installed automatically, it may be more convenient. Unsure if some of currently supported linux distributions have notify-send that can not handle title as the first argument.
> 
>> -	((fboundp 'notifications-notify)
>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
> 
> Does it mean that `notifications-notify' is bound but it does not work on macOS? If so, maybe it is better to put new clause for 'darwin above and to drop 'gnu/linux here. From my point of view, it is preferable to avoid additional requirement for `notifications-notify'. If someone will create a feature request for `notifications-notify' for macOS, it will just work without installing of additional packages as soon as such feature is implemented.
> 
> 
I indeed tried `notifications-notify`. And it does not work, reports error that it needs dbus. PS. I used the Homebrew formulae version Emacs.
I considered the order of conditions. Because notifications and notify-send etc requires dbus. So I guess only Linux supports that. So add system-type detection will be better. WDYT?

Re: [PATCH] make org-notify support for macOS desktop notification

[Tim Cross]

stardiviner <numbchild@gmail.com> writes:

>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>> 
>> On 05/07/2021 10:50, stardiviner wrote:
>>> I updated the patch, I found the package `osx-lib` contains solution.
>>> So I removed the directly osascript process invocation.
>> 
>> I have no objections any more. On the other hand I have no access to macOS, so
>> I have not tested this patch. Feel free to ignore comments from this message,
>> they are mostly matter of taste.
>> 
>> I expect that a simple script "notify-send" may allow to avoid modification of
>> code. Something like (untested, unsure concerning "quoted form of ...")
>> 
>> #!/usr/bin/env osascript
>> display notification (item 1 of argv)
>> 
>> However if osx-lib in is installed automatically, it may be more convenient.
>> Unsure if some of currently supported linux distributions have notify-send
>> that can not handle title as the first argument.
>> 
>>> -	((fboundp 'notifications-notify)
>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>> 
>> Does it mean that `notifications-notify' is bound but it does not work on
>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>> additional requirement for `notifications-notify'. If someone will create a
>> feature request for `notifications-notify' for macOS, it will just work
>> without installing of additional packages as soon as such feature is
>> implemented.
>> 
>> 
> I indeed tried `notifications-notify`. And it does not work, reports error that
> it needs dbus. PS. I used the Homebrew formulae version Emacs.
> I considered the order of conditions. Because notifications and notify-send etc
> requires dbus. So I guess only Linux supports that. So add system-type detection
> will be better. WDYT?

I think you can add dbus support to macOS using homebrew and that might
resolve the issue. At the very least, this will need to be investigated
because otherwise, adding this patch may break configurations for users
who have added dbus support via homebrew and have notifications working,
but have not installed the osx-lib package.

My only small concern with your proposed changes is that it will add a
dependency on a new package osx-lib, which I think is only available in
melpa. At the very least, this will need to be documented somewhere.
However, I'm not sure what the situation is wrt adding code which
depends on an external package which is not available in either elpa or
nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
which 'encourages' the use of melpa packages will not be acceptable. 

-- 
Tim Cross

Re: [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com> wrote:
> 
> 
> stardiviner <numbchild@gmail.com> writes:
> 
>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>>> 
>>> On 05/07/2021 10:50, stardiviner wrote:
>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>> So I removed the directly osascript process invocation.
>>> 
>>> I have no objections any more. On the other hand I have no access to macOS, so
>>> I have not tested this patch. Feel free to ignore comments from this message,
>>> they are mostly matter of taste.
>>> 
>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>> 
>>> #!/usr/bin/env osascript
>>> display notification (item 1 of argv)
>>> 
>>> However if osx-lib in is installed automatically, it may be more convenient.
>>> Unsure if some of currently supported linux distributions have notify-send
>>> that can not handle title as the first argument.
>>> 
>>>> -	((fboundp 'notifications-notify)
>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>> 
>>> Does it mean that `notifications-notify' is bound but it does not work on
>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>> additional requirement for `notifications-notify'. If someone will create a
>>> feature request for `notifications-notify' for macOS, it will just work
>>> without installing of additional packages as soon as such feature is
>>> implemented.
>>> 
>>> 
>> I indeed tried `notifications-notify`. And it does not work, reports error that
>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>> I considered the order of conditions. Because notifications and notify-send etc
>> requires dbus. So I guess only Linux supports that. So add system-type detection
>> will be better. WDYT?
> 
> I think you can add dbus support to macOS using homebrew and that might
> resolve the issue.

I checked my homebrew, I found the `dbus` is installed already. And in Emacs `(featurep ‘dbus)` returns t.

But `(org-show-notification “test”)` returns error:
```
Debugger entered--Lisp error: (dbus-error "No connection to bus" :session)
  dbus-message-internal(1 :session "org.freedesktop.Notifications" "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify" dbus-call-method-handler :string "Emacs" :uint32 0 :string "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0))) :int32 3000)
  apply(dbus-message-internal 1 :session "org.freedesktop.Notifications" "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify" dbus-call-method-handler (:string "Emacs" :uint32 0 :string "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0))) :int32 3000))
  dbus-call-method(:session "org.freedesktop.Notifications" "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify" :string "Emacs" :uint32 0 :string "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0))) :int32 3000)
  notifications-notify(:title "Org mode message" :body "test" :timeout 3000 :urgency low)
  (cond ((functionp org-show-notification-handler) (funcall org-show-notification-handler notification)) ((stringp org-show-notification-handler) (start-process "emacs-timer-notification" nil org-show-notification-handler notification)) ((fboundp 'w32-notification-notify) (let ((id (w32-notification-notify :title "Org mode message" :body notification :urgency 'low))) (run-with-timer org-show-notification-timeout nil #'(lambda nil (w32-notification-close id))))) ((fboundp 'notifications-notify) (notifications-notify :title "Org mode message" :body notification :timeout (* org-show-notification-timeout 1000) :urgency 'low)) ((executable-find "notify-send") (start-process "emacs-timer-notification" nil "notify-send" notification)) (t (message "%s" notification)))
  org-show-notification("test")
  eval((org-show-notification "test") nil)
  elisp--eval-last-sexp(nil)
  eval-last-sexp(nil)
  funcall-interactively(eval-last-sexp nil)
  call-interactively(eval-last-sexp nil nil)
  command-execute(eval-last-sexp)

```

Seems Emacs can’t connect to Homebrew dbus session. But I checked homebrew services, the dbus service is running.

Then I googled this problem. But have not found solution.

Does anyone has experience to solve this problem?

But lucky I found another way to invoke AppleScript notification way built-in Emacs.

#+begin_src emacs-lisp
(ns-do-applescript "display notification \"hello world\"")

(ns-do-applescript "display notification \"hello world\" with title \"some title\"")
#+end_src

I will update the patch in next email.

> At the very least, this will need to be investigated
> because otherwise, adding this patch may break configurations for users
> who have added dbus support via homebrew and have notifications working,
> but have not installed the osx-lib package.
> 
> My only small concern with your proposed changes is that it will add a
> dependency on a new package osx-lib, which I think is only available in
> melpa. At the very least, this will need to be documented somewhere.
> However, I'm not sure what the situation is wrt adding code which
> depends on an external package which is not available in either elpa or
> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
> which 'encourages' the use of melpa packages will not be acceptable. 
> 
> -- 
> Tim Cross
> 

Re: [PATCH] make org-notify support for macOS desktop notification

[Tim Cross]

stardiviner <numbchild@gmail.com> writes:

>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com> wrote:
>> 
>> 
>> stardiviner <numbchild@gmail.com> writes:
>> 
>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>>>> 
>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>> So I removed the directly osascript process invocation.
>>>> 
>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>> they are mostly matter of taste.
>>>> 
>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>> 
>>>> #!/usr/bin/env osascript
>>>> display notification (item 1 of argv)
>>>> 
>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>> Unsure if some of currently supported linux distributions have notify-send
>>>> that can not handle title as the first argument.
>>>> 
>>>>> -	((fboundp 'notifications-notify)
>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>> 
>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>> additional requirement for `notifications-notify'. If someone will create a
>>>> feature request for `notifications-notify' for macOS, it will just work
>>>> without installing of additional packages as soon as such feature is
>>>> implemented.
>>>> 
>>>> 
>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>> I considered the order of conditions. Because notifications and notify-send etc
>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>> will be better. WDYT?
>> 
>> I think you can add dbus support to macOS using homebrew and that might
>> resolve the issue.
>
> I checked my homebrew, I found the `dbus` is installed already. And in Emacs `(featurep ‘dbus)` returns t.
>
> But `(org-show-notification “test”)` returns error:
> ```
> Debugger entered--Lisp error: (dbus-error "No connection to bus" :session)
>   dbus-message-internal(1 :session "org.freedesktop.Notifications"
> "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify"
> dbus-call-method-handler :string "Emacs" :uint32 0 :string
> "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode
> message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0)))
> :int32 3000)
>   apply(dbus-message-internal 1 :session "org.freedesktop.Notifications"
> "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify"
> dbus-call-method-handler (:string "Emacs" :uint32 0 :string
> "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode
> message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0)))
> :int32 3000))
>   dbus-call-method(:session "org.freedesktop.Notifications"
> "/org/freedesktop/Notifications" "org.freedesktop.Notifications" "Notify"
> :string "Emacs" :uint32 0 :string
> "/opt/homebrew/Cellar/emacs-head@28/28.0.50_1/share..." :string "Org mode
> message" :string "test" (:array) ((:dict-entry "urgency" (:variant :byte 0)))
> :int32 3000)
>   notifications-notify(:title "Org mode message" :body "test" :timeout 3000 :urgency low)
>   (cond ((functionp org-show-notification-handler) (funcall
> org-show-notification-handler notification)) ((stringp
> org-show-notification-handler) (start-process "emacs-timer-notification" nil
> org-show-notification-handler notification)) ((fboundp 'w32-notification-notify)
> (let ((id (w32-notification-notify :title "Org mode message" :body notification
> :urgency 'low))) (run-with-timer org-show-notification-timeout nil #'(lambda nil
> (w32-notification-close id))))) ((fboundp 'notifications-notify)
> (notifications-notify :title "Org mode message" :body notification :timeout (*
> org-show-notification-timeout 1000) :urgency 'low)) ((executable-find
> "notify-send") (start-process "emacs-timer-notification" nil "notify-send"
> notification)) (t (message "%s" notification)))
>   org-show-notification("test")
>   eval((org-show-notification "test") nil)
>   elisp--eval-last-sexp(nil)
>   eval-last-sexp(nil)
>   funcall-interactively(eval-last-sexp nil)
>   call-interactively(eval-last-sexp nil nil)
>   command-execute(eval-last-sexp)
>
> ```
>
> Seems Emacs can’t connect to Homebrew dbus session. But I checked homebrew services, the dbus service is running.

Probably some permission issue. I'll try to boot up my old mac on the
weekend and see if I can get it working. 

The other thing which also needs to be considered is whether your patch
will have any adverse impact for users of the mac port of Emacs. I've
been running the railwaycat emacs mac port formula from brew for ages
now. It works much better than vanilla emacs on macOS. Don't know what
the situation is with either dbus or integration with macOS
notifications - I seem to recall something about getting closer
integration using the mac ports version.

I resisted the mac port of Emacs for years, believing it was better to
stick to stock standard Emacs. However, when I did decide to try it out,
I was surprised how much better it worked. Lots of little niggles I had
with the standard Emacs under OSX/macOS just vanished. In fact, the
biggest issue I had to deal with was removing all my hacky kludges I'd
added over the years to make Emacs work well on the mac. Highly
recommend it.

-- 
Tim Cross

[new patch] Re: [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 100 bytes --]

Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.


[-- Attachment #2: 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch --]
[-- Type: application/octet-stream, Size: 1047 bytes --]

From 9dcf98905cd2d11c899f8a13238601345a6361cc Mon Sep 17 00:00:00 2001
From: stardiviner <numbchild@gmail.com>
Date: Sun, 4 Jul 2021 08:13:43 +0800
Subject: [PATCH] org-clock.el: Make org-notify support macOS notification

* lisp/org-clock.el (org-show-notification): Add support for macOS
notification.
---
 lisp/org-clock.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index cd930e875..efc3a2f3b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -859,6 +859,10 @@ use libnotify if available, or fall back on a message."
 	    org-show-notification-timeout
 	    nil
 	    (lambda () (w32-notification-close id)))))
+        ((fboundp 'ns-do-applescript)
+         (ns-do-applescript
+          (format "display notification \"%s\" with title \"Org mode notification\""
+                  (replace-regexp-in-string "\"" "#" notification))))
 	((fboundp 'notifications-notify)
 	 (notifications-notify
 	  :title "Org mode message"
-- 
2.30.1 (Apple Git-130)


[-- Attachment #3: Type: text/plain, Size: 2948 bytes --]



> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com> wrote:
> 
> 
> stardiviner <numbchild@gmail.com> writes:
> 
>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>>> 
>>> On 05/07/2021 10:50, stardiviner wrote:
>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>> So I removed the directly osascript process invocation.
>>> 
>>> I have no objections any more. On the other hand I have no access to macOS, so
>>> I have not tested this patch. Feel free to ignore comments from this message,
>>> they are mostly matter of taste.
>>> 
>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>> 
>>> #!/usr/bin/env osascript
>>> display notification (item 1 of argv)
>>> 
>>> However if osx-lib in is installed automatically, it may be more convenient.
>>> Unsure if some of currently supported linux distributions have notify-send
>>> that can not handle title as the first argument.
>>> 
>>>> -	((fboundp 'notifications-notify)
>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>> 
>>> Does it mean that `notifications-notify' is bound but it does not work on
>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>> additional requirement for `notifications-notify'. If someone will create a
>>> feature request for `notifications-notify' for macOS, it will just work
>>> without installing of additional packages as soon as such feature is
>>> implemented.
>>> 
>>> 
>> I indeed tried `notifications-notify`. And it does not work, reports error that
>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>> I considered the order of conditions. Because notifications and notify-send etc
>> requires dbus. So I guess only Linux supports that. So add system-type detection
>> will be better. WDYT?
> 
> I think you can add dbus support to macOS using homebrew and that might
> resolve the issue. At the very least, this will need to be investigated
> because otherwise, adding this patch may break configurations for users
> who have added dbus support via homebrew and have notifications working,
> but have not installed the osx-lib package.
> 
> My only small concern with your proposed changes is that it will add a
> dependency on a new package osx-lib, which I think is only available in
> melpa. At the very least, this will need to be documented somewhere.
> However, I'm not sure what the situation is wrt adding code which
> depends on an external package which is not available in either elpa or
> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
> which 'encourages' the use of melpa packages will not be acceptable. 
> 
> -- 
> Tim Cross
> 

Re: [new patch] Re: [PATCH] make org-notify support for macOS desktop notification

[Christian Hopps]

Hi,

Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".

Info can be found here:

  https://github.com/choppsv1/homebrew-emacsmacport

Thanks,
Chris.

stardiviner <numbchild@gmail.com> writes:

> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>
> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>
>
>
>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com> wrote:
>>
>>
>> stardiviner <numbchild@gmail.com> writes:
>>
>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>>>>
>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>> So I removed the directly osascript process invocation.
>>>>
>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>> they are mostly matter of taste.
>>>>
>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>
>>>> #!/usr/bin/env osascript
>>>> display notification (item 1 of argv)
>>>>
>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>> Unsure if some of currently supported linux distributions have notify-send
>>>> that can not handle title as the first argument.
>>>>
>>>>> -	((fboundp 'notifications-notify)
>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>
>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>> additional requirement for `notifications-notify'. If someone will create a
>>>> feature request for `notifications-notify' for macOS, it will just work
>>>> without installing of additional packages as soon as such feature is
>>>> implemented.
>>>>
>>>>
>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>> I considered the order of conditions. Because notifications and notify-send etc
>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>> will be better. WDYT?
>>
>> I think you can add dbus support to macOS using homebrew and that might
>> resolve the issue. At the very least, this will need to be investigated
>> because otherwise, adding this patch may break configurations for users
>> who have added dbus support via homebrew and have notifications working,
>> but have not installed the osx-lib package.
>>
>> My only small concern with your proposed changes is that it will add a
>> dependency on a new package osx-lib, which I think is only available in
>> melpa. At the very least, this will need to be documented somewhere.
>> However, I'm not sure what the situation is wrt adding code which
>> depends on an external package which is not available in either elpa or
>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>> which 'encourages' the use of melpa packages will not be acceptable.
>>
>> --
>> Tim Cross
>>

Re: [new patch] [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 4064 bytes --]

Thanks for your suggestion. Does your Emacs build supports imagemagick image view and svg feature support? Because company-mode now have built-in icons support. This is the reason that I switch from https://emacsformacosx.com/ <https://emacsformacosx.com/> to Homebrew cask Emacs version.

> On Jul 6, 2021, at 12:21 PM, Christian Hopps <chopps@chopps.org> wrote:
> 
> Hi,
> 
> Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".
> 
> Info can be found here:
> 
> https://github.com/choppsv1/homebrew-emacsmacport
> 
> Thanks,
> Chris.
> 
> stardiviner <numbchild@gmail.com> writes:
> 
>> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>> 
>> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>> 
>> 
>> 
>>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com> wrote:
>>> 
>>> 
>>> stardiviner <numbchild@gmail.com> writes:
>>> 
>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com> wrote:
>>>>> 
>>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>>> So I removed the directly osascript process invocation.
>>>>> 
>>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>>> they are mostly matter of taste.
>>>>> 
>>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>> 
>>>>> #!/usr/bin/env osascript
>>>>> display notification (item 1 of argv)
>>>>> 
>>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>>> Unsure if some of currently supported linux distributions have notify-send
>>>>> that can not handle title as the first argument.
>>>>> 
>>>>>> -	((fboundp 'notifications-notify)
>>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>> 
>>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>>> additional requirement for `notifications-notify'. If someone will create a
>>>>> feature request for `notifications-notify' for macOS, it will just work
>>>>> without installing of additional packages as soon as such feature is
>>>>> implemented.
>>>>> 
>>>>> 
>>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>>> I considered the order of conditions. Because notifications and notify-send etc
>>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>>> will be better. WDYT?
>>> 
>>> I think you can add dbus support to macOS using homebrew and that might
>>> resolve the issue. At the very least, this will need to be investigated
>>> because otherwise, adding this patch may break configurations for users
>>> who have added dbus support via homebrew and have notifications working,
>>> but have not installed the osx-lib package.
>>> 
>>> My only small concern with your proposed changes is that it will add a
>>> dependency on a new package osx-lib, which I think is only available in
>>> melpa. At the very least, this will need to be documented somewhere.
>>> However, I'm not sure what the situation is wrt adding code which
>>> depends on an external package which is not available in either elpa or
>>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>>> which 'encourages' the use of melpa packages will not be acceptable.
>>> 
>>> --
>>> Tim Cross
>>> 
> 


[-- Attachment #2: Type: text/html, Size: 5752 bytes --]

Re: [new patch] [PATCH] make org-notify support for macOS desktop notification

[Christian Hopps]

[-- Attachment #1: Type: text/plain, Size: 4616 bytes --]

It supports imagemagick (specify —with-imagemagick), and it includes svg by default, I simply forked the railwaycat version and added the native notification code.

Thanks,
Chris.

> On Jul 6, 2021, at 11:30 AM, stardiviner <numbchild@gmail.com> wrote:
> 
> Thanks for your suggestion. Does your Emacs build supports imagemagick image view and svg feature support? Because company-mode now have built-in icons support. This is the reason that I switch from https://emacsformacosx.com/ <https://emacsformacosx.com/> to Homebrew cask Emacs version.
> 
>> On Jul 6, 2021, at 12:21 PM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>> 
>> Hi,
>> 
>> Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".
>> 
>> Info can be found here:
>> 
>> https://github.com/choppsv1/homebrew-emacsmacport <https://github.com/choppsv1/homebrew-emacsmacport>
>> 
>> Thanks,
>> Chris.
>> 
>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>> 
>>> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>>> 
>>> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>>> 
>>> 
>>> 
>>>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com <mailto:theophilusx@gmail.com>> wrote:
>>>> 
>>>> 
>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>> 
>>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com <mailto:manikulin@gmail.com>> wrote:
>>>>>> 
>>>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>>>> So I removed the directly osascript process invocation.
>>>>>> 
>>>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>>>> they are mostly matter of taste.
>>>>>> 
>>>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>>> 
>>>>>> #!/usr/bin/env osascript
>>>>>> display notification (item 1 of argv)
>>>>>> 
>>>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>>>> Unsure if some of currently supported linux distributions have notify-send
>>>>>> that can not handle title as the first argument.
>>>>>> 
>>>>>>> -	((fboundp 'notifications-notify)
>>>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>>> 
>>>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>>>> additional requirement for `notifications-notify'. If someone will create a
>>>>>> feature request for `notifications-notify' for macOS, it will just work
>>>>>> without installing of additional packages as soon as such feature is
>>>>>> implemented.
>>>>>> 
>>>>>> 
>>>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>>>> I considered the order of conditions. Because notifications and notify-send etc
>>>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>>>> will be better. WDYT?
>>>> 
>>>> I think you can add dbus support to macOS using homebrew and that might
>>>> resolve the issue. At the very least, this will need to be investigated
>>>> because otherwise, adding this patch may break configurations for users
>>>> who have added dbus support via homebrew and have notifications working,
>>>> but have not installed the osx-lib package.
>>>> 
>>>> My only small concern with your proposed changes is that it will add a
>>>> dependency on a new package osx-lib, which I think is only available in
>>>> melpa. At the very least, this will need to be documented somewhere.
>>>> However, I'm not sure what the situation is wrt adding code which
>>>> depends on an external package which is not available in either elpa or
>>>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>>>> which 'encourages' the use of melpa packages will not be acceptable.
>>>> 
>>>> --
>>>> Tim Cross
>>>> 
>> 
> 


[-- Attachment #2: Type: text/html, Size: 6495 bytes --]

Re: [new patch] [PATCH] make org-notify support for macOS desktop notification

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 5054 bytes --]

Hi Chris, thanks for your work. I have a question, will your patch of notification code be merged to upstream?
If yes, I think my patch will be not necessary. If no, then I think add a my workaround for macOS is considerable.

> On Jul 7, 2021, at 2:23 AM, Christian Hopps <chopps@chopps.org> wrote:
> 
> It supports imagemagick (specify —with-imagemagick), and it includes svg by default, I simply forked the railwaycat version and added the native notification code.
> 
> Thanks,
> Chris.
> 
>> On Jul 6, 2021, at 11:30 AM, stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> wrote:
>> 
>> Thanks for your suggestion. Does your Emacs build supports imagemagick image view and svg feature support? Because company-mode now have built-in icons support. This is the reason that I switch from https://emacsformacosx.com/ <https://emacsformacosx.com/> to Homebrew cask Emacs version.
>> 
>>> On Jul 6, 2021, at 12:21 PM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>>> 
>>> Hi,
>>> 
>>> Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".
>>> 
>>> Info can be found here:
>>> 
>>> https://github.com/choppsv1/homebrew-emacsmacport <https://github.com/choppsv1/homebrew-emacsmacport>
>>> 
>>> Thanks,
>>> Chris.
>>> 
>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>> 
>>>> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>>>> 
>>>> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>>>> 
>>>> 
>>>> 
>>>>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com <mailto:theophilusx@gmail.com>> wrote:
>>>>> 
>>>>> 
>>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>>> 
>>>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com <mailto:manikulin@gmail.com>> wrote:
>>>>>>> 
>>>>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>>>>> So I removed the directly osascript process invocation.
>>>>>>> 
>>>>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>>>>> they are mostly matter of taste.
>>>>>>> 
>>>>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>>>> 
>>>>>>> #!/usr/bin/env osascript
>>>>>>> display notification (item 1 of argv)
>>>>>>> 
>>>>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>>>>> Unsure if some of currently supported linux distributions have notify-send
>>>>>>> that can not handle title as the first argument.
>>>>>>> 
>>>>>>>> -	((fboundp 'notifications-notify)
>>>>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>>>> 
>>>>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>>>>> additional requirement for `notifications-notify'. If someone will create a
>>>>>>> feature request for `notifications-notify' for macOS, it will just work
>>>>>>> without installing of additional packages as soon as such feature is
>>>>>>> implemented.
>>>>>>> 
>>>>>>> 
>>>>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>>>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>>>>> I considered the order of conditions. Because notifications and notify-send etc
>>>>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>>>>> will be better. WDYT?
>>>>> 
>>>>> I think you can add dbus support to macOS using homebrew and that might
>>>>> resolve the issue. At the very least, this will need to be investigated
>>>>> because otherwise, adding this patch may break configurations for users
>>>>> who have added dbus support via homebrew and have notifications working,
>>>>> but have not installed the osx-lib package.
>>>>> 
>>>>> My only small concern with your proposed changes is that it will add a
>>>>> dependency on a new package osx-lib, which I think is only available in
>>>>> melpa. At the very least, this will need to be documented somewhere.
>>>>> However, I'm not sure what the situation is wrt adding code which
>>>>> depends on an external package which is not available in either elpa or
>>>>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>>>>> which 'encourages' the use of melpa packages will not be acceptable.
>>>>> 
>>>>> --
>>>>> Tim Cross
>>>>> 
>>> 
>> 
> 


[-- Attachment #2: Type: text/html, Size: 7230 bytes --]

Re: [new patch] [PATCH] make org-notify support for macOS desktop notification

[Christian Hopps]

[-- Attachment #1: Type: text/plain, Size: 6643 bytes --]

It may eventually be incorporated into the very popular emacs-mac port (railwaycat tap in homebrew); however, it will probably not be incorporated into the nextstep/emacs main code. I started looking at doing a version for the mainline code, but it’s hard to get motivated b/c using that version of emacs on OS X is a pretty sub-par experience.

I only commented on this b/c I think you might are disabling notifications-notify which work great with my code changes, and using something else if you see Darwin OS, and that will break my native “Just Works” support for notifications, which again may end up on many peoples machines. I would ask that the patch be modified in a way that didn’t break native support if present before it was accepted.

Also as you can see by the multiple patches you’ve submitted there’s really no good answer for an external notifier, so whatever you pick is probably going to be wrong for someone I guess.

If this patch is going to be accepted I would ask that it

1) be conditional (disable-able with a variable)
2) do the check for the custom installed external notifier and if not present then fallback to using the emacs supplied notifications-notify
3) not restrict notifications-notify to gnu/linux only.

That way people that have already developed solutions for this won’t have them broken.

Thanks,
Chris.

> On Jul 7, 2021, at 8:00 PM, stardiviner <numbchild@gmail.com> wrote:
> 
> Hi Chris, thanks for your work. I have a question, will your patch of notification code be merged to upstream?
> If yes, I think my patch will be not necessary. If no, then I think add a my workaround for macOS is considerable.
> 
>> On Jul 7, 2021, at 2:23 AM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>> 
>> It supports imagemagick (specify —with-imagemagick), and it includes svg by default, I simply forked the railwaycat version and added the native notification code.
>> 
>> Thanks,
>> Chris.
>> 
>>> On Jul 6, 2021, at 11:30 AM, stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> wrote:
>>> 
>>> Thanks for your suggestion. Does your Emacs build supports imagemagick image view and svg feature support? Because company-mode now have built-in icons support. This is the reason that I switch from https://emacsformacosx.com/ <https://emacsformacosx.com/> to Homebrew cask Emacs version.
>>> 
>>>> On Jul 6, 2021, at 12:21 PM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".
>>>> 
>>>> Info can be found here:
>>>> 
>>>> https://github.com/choppsv1/homebrew-emacsmacport <https://github.com/choppsv1/homebrew-emacsmacport>
>>>> 
>>>> Thanks,
>>>> Chris.
>>>> 
>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>> 
>>>>> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>>>>> 
>>>>> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com <mailto:theophilusx@gmail.com>> wrote:
>>>>>> 
>>>>>> 
>>>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>>>> 
>>>>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com <mailto:manikulin@gmail.com>> wrote:
>>>>>>>> 
>>>>>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>>>>>> So I removed the directly osascript process invocation.
>>>>>>>> 
>>>>>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>>>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>>>>>> they are mostly matter of taste.
>>>>>>>> 
>>>>>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>>>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>>>>> 
>>>>>>>> #!/usr/bin/env osascript
>>>>>>>> display notification (item 1 of argv)
>>>>>>>> 
>>>>>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>>>>>> Unsure if some of currently supported linux distributions have notify-send
>>>>>>>> that can not handle title as the first argument.
>>>>>>>> 
>>>>>>>>> -	((fboundp 'notifications-notify)
>>>>>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>>>>> 
>>>>>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>>>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>>>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>>>>>> additional requirement for `notifications-notify'. If someone will create a
>>>>>>>> feature request for `notifications-notify' for macOS, it will just work
>>>>>>>> without installing of additional packages as soon as such feature is
>>>>>>>> implemented.
>>>>>>>> 
>>>>>>>> 
>>>>>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>>>>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>>>>>> I considered the order of conditions. Because notifications and notify-send etc
>>>>>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>>>>>> will be better. WDYT?
>>>>>> 
>>>>>> I think you can add dbus support to macOS using homebrew and that might
>>>>>> resolve the issue. At the very least, this will need to be investigated
>>>>>> because otherwise, adding this patch may break configurations for users
>>>>>> who have added dbus support via homebrew and have notifications working,
>>>>>> but have not installed the osx-lib package.
>>>>>> 
>>>>>> My only small concern with your proposed changes is that it will add a
>>>>>> dependency on a new package osx-lib, which I think is only available in
>>>>>> melpa. At the very least, this will need to be documented somewhere.
>>>>>> However, I'm not sure what the situation is wrt adding code which
>>>>>> depends on an external package which is not available in either elpa or
>>>>>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>>>>>> which 'encourages' the use of melpa packages will not be acceptable.
>>>>>> 
>>>>>> --
>>>>>> Tim Cross
>>>>>> 
>>>> 
>>> 
>> 
> 


[-- Attachment #2: Type: text/html, Size: 9446 bytes --]

STOP this patch for now.

[stardiviner]

[-- Attachment #1: Type: text/plain, Size: 7186 bytes --]



> On Jul 8, 2021, at 4:59 PM, Christian Hopps <chopps@chopps.org> wrote:
> 
> It may eventually be incorporated into the very popular emacs-mac port (railwaycat tap in homebrew); however, it will probably not be incorporated into the nextstep/emacs main code. I started looking at doing a version for the mainline code, but it’s hard to get motivated b/c using that version of emacs on OS X is a pretty sub-par experience.

Thanks for your work on this support. I found upstream is less active. Don’t know when will be merged.

> 
> I only commented on this b/c I think you might are disabling notifications-notify which work great with my code changes, and using something else if you see Darwin OS, and that will break my native “Just Works” support for notifications, which again may end up on many peoples machines. I would ask that the patch be modified in a way that didn’t break native support if present before it was accepted.
> 
> Also as you can see by the multiple patches you’ve submitted there’s really no good answer for an external notifier, so whatever you pick is probably going to be wrong for someone I guess.

Yes, this troubled my too. Currently no good solution. I will stop this patch for now. Wait for upstream emacs-mac port support. Hope it will be arrived in at leas half of year.

> 
> If this patch is going to be accepted I would ask that it
> 
> 1) be conditional (disable-able with a variable)
> 2) do the check for the custom installed external notifier and if not present then fallback to using the emacs supplied notifications-notify
> 3) not restrict notifications-notify to gnu/linux only.
> 
> That way people that have already developed solutions for this won’t have them broken.
> 
> Thanks,
> Chris.
> 
>> On Jul 7, 2021, at 8:00 PM, stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> wrote:
>> 
>> Hi Chris, thanks for your work. I have a question, will your patch of notification code be merged to upstream?
>> If yes, I think my patch will be not necessary. If no, then I think add a my workaround for macOS is considerable.
>> 
>>> On Jul 7, 2021, at 2:23 AM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>>> 
>>> It supports imagemagick (specify —with-imagemagick), and it includes svg by default, I simply forked the railwaycat version and added the native notification code.
>>> 
>>> Thanks,
>>> Chris.
>>> 
>>>> On Jul 6, 2021, at 11:30 AM, stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> wrote:
>>>> 
>>>> Thanks for your suggestion. Does your Emacs build supports imagemagick image view and svg feature support? Because company-mode now have built-in icons support. This is the reason that I switch from https://emacsformacosx.com/ <https://emacsformacosx.com/> to Homebrew cask Emacs version.
>>>> 
>>>>> On Jul 6, 2021, at 12:21 PM, Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> Please consider: I added full native notification support to the popular OS X Emacs build available in homebrew. This supports rewrites notifications-notify defun to use the native code rather than dbus, and so everything "Just Works".
>>>>> 
>>>>> Info can be found here:
>>>>> 
>>>>> https://github.com/choppsv1/homebrew-emacsmacport <https://github.com/choppsv1/homebrew-emacsmacport>
>>>>> 
>>>>> Thanks,
>>>>> Chris.
>>>>> 
>>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>>> 
>>>>>> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.
>>>>>> 
>>>>>> [2. text/x-patch; 0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> On Jul 6, 2021, at 8:06 AM, Tim Cross <theophilusx@gmail.com <mailto:theophilusx@gmail.com>> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> stardiviner <numbchild@gmail.com <mailto:numbchild@gmail.com>> writes:
>>>>>>> 
>>>>>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <manikulin@gmail.com <mailto:manikulin@gmail.com>> wrote:
>>>>>>>>> 
>>>>>>>>> On 05/07/2021 10:50, stardiviner wrote:
>>>>>>>>>> I updated the patch, I found the package `osx-lib` contains solution.
>>>>>>>>>> So I removed the directly osascript process invocation.
>>>>>>>>> 
>>>>>>>>> I have no objections any more. On the other hand I have no access to macOS, so
>>>>>>>>> I have not tested this patch. Feel free to ignore comments from this message,
>>>>>>>>> they are mostly matter of taste.
>>>>>>>>> 
>>>>>>>>> I expect that a simple script "notify-send" may allow to avoid modification of
>>>>>>>>> code. Something like (untested, unsure concerning "quoted form of ...")
>>>>>>>>> 
>>>>>>>>> #!/usr/bin/env osascript
>>>>>>>>> display notification (item 1 of argv)
>>>>>>>>> 
>>>>>>>>> However if osx-lib in is installed automatically, it may be more convenient.
>>>>>>>>> Unsure if some of currently supported linux distributions have notify-send
>>>>>>>>> that can not handle title as the first argument.
>>>>>>>>> 
>>>>>>>>>> -	((fboundp 'notifications-notify)
>>>>>>>>>> +	((and (eq system-type 'gnu/linux) (fboundp 'notifications-notify))
>>>>>>>>> 
>>>>>>>>> Does it mean that `notifications-notify' is bound but it does not work on
>>>>>>>>> macOS? If so, maybe it is better to put new clause for 'darwin above and to
>>>>>>>>> drop 'gnu/linux here. From my point of view, it is preferable to avoid
>>>>>>>>> additional requirement for `notifications-notify'. If someone will create a
>>>>>>>>> feature request for `notifications-notify' for macOS, it will just work
>>>>>>>>> without installing of additional packages as soon as such feature is
>>>>>>>>> implemented.
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> I indeed tried `notifications-notify`. And it does not work, reports error that
>>>>>>>> it needs dbus. PS. I used the Homebrew formulae version Emacs.
>>>>>>>> I considered the order of conditions. Because notifications and notify-send etc
>>>>>>>> requires dbus. So I guess only Linux supports that. So add system-type detection
>>>>>>>> will be better. WDYT?
>>>>>>> 
>>>>>>> I think you can add dbus support to macOS using homebrew and that might
>>>>>>> resolve the issue. At the very least, this will need to be investigated
>>>>>>> because otherwise, adding this patch may break configurations for users
>>>>>>> who have added dbus support via homebrew and have notifications working,
>>>>>>> but have not installed the osx-lib package.
>>>>>>> 
>>>>>>> My only small concern with your proposed changes is that it will add a
>>>>>>> dependency on a new package osx-lib, which I think is only available in
>>>>>>> melpa. At the very least, this will need to be documented somewhere.
>>>>>>> However, I'm not sure what the situation is wrt adding code which
>>>>>>> depends on an external package which is not available in either elpa or
>>>>>>> nongnuELPA? As org mode is a part of GNU Emacs, I suspect that any code
>>>>>>> which 'encourages' the use of melpa packages will not be acceptable.
>>>>>>> 
>>>>>>> --
>>>>>>> Tim Cross
>>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 


[-- Attachment #2: Type: text/html, Size: 10673 bytes --]

Re: STOP this patch for now.

[Christian Hopps]

[-- Attachment #1: Type: text/plain, Size: 11081 bytes --]


stardiviner <numbchild@gmail.com> writes:

>     On Jul 8, 2021, at 4:59 PM, Christian Hopps <chopps@chopps.org>
>     wrote:
>
>     It may eventually be incorporated into the very popular emacs-mac
>     port (railwaycat tap in homebrew); however, it will probably not
>     be incorporated into the nextstep/emacs main code. I started
>     looking at doing a version for the mainline code, but it’s hard
>     to get motivated b/c using that version of emacs on OS X is a
>     pretty sub-par experience.
>
>
> Thanks for your work on this support. I found upstream is less
> active. Don’t know when will be merged.
>
>
>
>     I only commented on this b/c I think you might are disabling
>     notifications-notify which work great with my code changes, and
>     using something else if you see Darwin OS, and that will break my
>     native “Just Works” support for notifications, which again may
>     end up on many peoples machines. I would ask that the patch be
>     modified in a way that didn’t break native support if present
>     before it was accepted.
>
>     Also as you can see by the multiple patches you’ve submitted
>     there’s really no good answer for an external notifier, so
>     whatever you pick is probably going to be wrong for someone I
>     guess.
>
>
> Yes, this troubled my too. Currently no good solution. I will stop
> this patch for now. Wait for upstream emacs-mac port support. Hope it
> will be arrived in at leas half of year.

You could add a comment to the pull-request in support of merging. :)

https://bitbucket.org/mituharu/emacs-mac/pull-requests/10

FWIW, if you are currently using the railwaycat tap in homebrew you can just switch to my tap and you'll get the support right now. It should be just as easy to install.

Thanks,
Chris.


>
>
>
>     If this patch is going to be accepted I would ask that it
>
>     1) be conditional (disable-able with a variable)
>     2) do the check for the custom installed external notifier and if
>     not present then fallback to using the emacs supplied
>     notifications-notify
>     3) not restrict notifications-notify to gnu/linux only.
>
>     That way people that have already developed solutions for this
>     won’t have them broken.
>
>     Thanks,
>     Chris.
>
>
>         On Jul 7, 2021, at 8:00 PM, stardiviner <numbchild@gmail.com>
>         wrote:
>
>         Hi Chris, thanks for your work. I have a question, will your
>         patch of notification code be merged to upstream?
>         If yes, I think my patch will be not necessary. If no, then I
>         think add a my workaround for macOS is considerable.
>
>
>             On Jul 7, 2021, at 2:23 AM, Christian Hopps <
>             chopps@chopps.org> wrote:
>
>             It supports imagemagick (specify —with-imagemagick), and
>             it includes svg by default, I simply forked the
>             railwaycat version and added the native notification
>             code.
>
>             Thanks,
>             Chris.
>
>
>                 On Jul 6, 2021, at 11:30 AM, stardiviner <
>                 numbchild@gmail.com> wrote:
>
>                 Thanks for your suggestion. Does your Emacs build
>                 supports imagemagick image view and svg feature
>                 support? Because company-mode now have built-in icons
>                 support. This is the reason that I switch from https:
>                 //emacsformacosx.com/ to Homebrew cask Emacs version.
>
>
>                     On Jul 6, 2021, at 12:21 PM, Christian Hopps <
>                     chopps@chopps.org> wrote:
>
>                     Hi,
>
>                     Please consider: I added full native notification
>                     support to the popular OS X Emacs build available
>                     in homebrew. This supports rewrites
>                     notifications-notify defun to use the native code
>                     rather than dbus, and so everything "Just Works".
>
>                     Info can be found here:
>
>                     https://github.com/choppsv1/homebrew-emacsmacport
>
>                     Thanks,
>                     Chris.
>
>                     stardiviner <numbchild@gmail.com> writes:
>
>
>                         Here is the new patch which invokes
>                         notifications though Emacs built-in API
>                         `ns-do-applescript`.
>
>                         [2. text/x-patch;
>                         0001-org-clock.el-Make-org-notify-support-macOS-notificat.patch]...
>
>
>
>
>                             On Jul 6, 2021, at 8:06 AM, Tim Cross <
>                             theophilusx@gmail.com> wrote:
>
>
>                             stardiviner <numbchild@gmail.com> writes:
>
>
>                                     On Jul 5, 2021, at 7:55 PM, Maxim
>                                     Nikulin <manikulin@gmail.com>
>                                     wrote:
>
>                                     On 05/07/2021 10:50, stardiviner
>                                     wrote:
>
>                                         I updated the patch, I found
>                                         the package `osx-lib`
>                                         contains solution.
>                                         So I removed the directly
>                                         osascript process invocation.
>
>
>                                     I have no objections any more. On
>                                     the other hand I have no access
>                                     to macOS, so
>                                     I have not tested this patch.
>                                     Feel free to ignore comments from
>                                     this message,
>                                     they are mostly matter of taste.
>
>                                     I expect that a simple script
>                                     "notify-send" may allow to avoid
>                                     modification of
>                                     code. Something like (untested,
>                                     unsure concerning "quoted form of
>                                     ...")
>
>                                     #!/usr/bin/env osascript
>                                     display notification (item 1 of
>                                     argv)
>
>                                     However if osx-lib in is
>                                     installed automatically, it may
>                                     be more convenient.
>                                     Unsure if some of currently
>                                     supported linux distributions
>                                     have notify-send
>                                     that can not handle title as the
>                                     first argument.
>
>
>                                         - ((fboundp
>                                         'notifications-notify)
>                                         + ((and (eq system-type 'gnu/
>                                         linux) (fboundp
>                                         'notifications-notify))
>
>
>                                     Does it mean that
>                                     `notifications-notify' is bound
>                                     but it does not work on
>                                     macOS? If so, maybe it is better
>                                     to put new clause for 'darwin
>                                     above and to
>                                     drop 'gnu/linux here. From my
>                                     point of view, it is preferable
>                                     to avoid
>                                     additional requirement for
>                                     `notifications-notify'. If
>                                     someone will create a
>                                     feature request for
>                                     `notifications-notify' for macOS,
>                                     it will just work
>                                     without installing of additional
>                                     packages as soon as such feature
>                                     is
>                                     implemented.
>
>
>
>                                 I indeed tried `notifications-notify
>                                 `. And it does not work, reports
>                                 error that
>                                 it needs dbus. PS. I used the
>                                 Homebrew formulae version Emacs.
>                                 I considered the order of conditions.
>                                 Because notifications and notify-send
>                                 etc
>                                 requires dbus. So I guess only Linux
>                                 supports that. So add system-type
>                                 detection
>                                 will be better. WDYT?
>
>
>                             I think you can add dbus support to macOS
>                             using homebrew and that might
>                             resolve the issue. At the very least,
>                             this will need to be investigated
>                             because otherwise, adding this patch may
>                             break configurations for users
>                             who have added dbus support via homebrew
>                             and have notifications working,
>                             but have not installed the osx-lib
>                             package.
>
>                             My only small concern with your proposed
>                             changes is that it will add a
>                             dependency on a new package osx-lib,
>                             which I think is only available in
>                             melpa. At the very least, this will need
>                             to be documented somewhere.
>                             However, I'm not sure what the situation
>                             is wrt adding code which
>                             depends on an external package which is
>                             not available in either elpa or
>                             nongnuELPA? As org mode is a part of GNU
>                             Emacs, I suspect that any code
>                             which 'encourages' the use of melpa
>                             packages will not be acceptable.
>
>                             --
>                             Tim Cross
>
>
>
>
>
>
>
>
>
>
>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] make org-notify support for macOS desktop notification

[Maxim Nikulin]

On 06/07/2021 08:45, stardiviner wrote:
> Here is the new patch which invokes notifications though Emacs built-in API `ns-do-applescript`.

I suspect, you considered your original patch as small and simple, 
didn't you?

> +          (format "display notification \"%s\" with title \"Org mode notification\""
> +                  (replace-regexp-in-string "\"" "#" notification))))

Last character of notification might be "\" so it might escape the 
closing quote. Some less common quote character like "´" may be less 
confusing than "#" in this confusing if proper escaping is a challenge. 
I am unsure how many people are still using 8-bit charsets.

It seems there is no way to pass argv through `ns-do-applescript'. Is 
there at least a helper to escape strings similar to 
`shell-quote-argument'? Otherwise it is not friendly to developers since 
everyone has to do it himself.

On 06/07/2021 07:06, Tim Cross wrote:
> I suspect that any code
> which 'encourages' the use of melpa packages will not be acceptabl
If there are license problems with `osx-lib-notify2', I expect, it can 
be utilized through `org-show-notification-handler'. However users have 
to manually add such customization that is not really convenient.

If `org-show-notification' depends on runtime configuration (whether 
DBus socket is available), maybe it is reasonable to rewrite the 
function and to add a convention that e.g. 'default return value causes 
attempt to call next handler in the responsibility chain. The intention 
is to allow fallback from `org-show-notification-handler' to standard 
notifiers or graceful degradation from `notifications-notify' to 
`message' if dbus connection is not available (e.g. Emacs is running in 
a minimal container). Such changes however out of scope of this patch.

Re: [new patch] Re: [PATCH] make org-notify support for macOS desktop notification

[Bastien]

Hi,

stardiviner <numbchild@gmail.com> writes:

> Here is the new patch which invokes notifications though Emacs
> built-in API `ns-do-applescript`.

Applied in the main branch, thanks.

-- 
 Bastien

Re: [new patch] Re: [PATCH] make org-notify support for macOS desktop notification

[Max Nikulin]

On 26/09/2021 15:52, Bastien wrote:
> stardiviner writes:
> 
>> Here is the new patch which invokes notifications though Emacs
>> built-in API `ns-do-applescript`.
> 
> Applied in the main branch, thanks.

Notice that applied patch has a minor issue: backslashes are not escaped 
before argument is passed to osa-script interpreter, so e.g. trailing 
backslash leads to syntax error
(https://list.orgmode.org/sc1hdn$12dt$1@ciao.gmane.io).

Disclaimer: I am not a macOS user.

 From discussion in this thread I had impression that patched for macOS 
versions work much better and they do not require this patch. Introduced 
clause has higher priority than notify-send however the latter works 
even on mac at least for some users.