emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Tim Cross <theophilusx@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: [PATCH] make org-notify support for macOS desktop notification
Date: Tue, 06 Jul 2021 10:06:29 +1000	[thread overview]
Message-ID: <87im1ol2lb.fsf@gmail.com> (raw)
In-Reply-To: <E67617DD-CE51-43CC-B585-DF9EB5CCDA84@gmail.com>


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


  reply	other threads:[~2021-07-06  0:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-04  0:23 stardiviner
2021-07-04  5:48 ` Maxim Nikulin
2021-07-05  3:50   ` stardiviner
2021-07-05 11:55     ` Maxim Nikulin
2021-07-05 22:36       ` stardiviner
2021-07-06  0:06         ` Tim Cross [this message]
2021-07-06  1:37           ` stardiviner
2021-07-06  8:12             ` Tim Cross
2021-07-06  1:45           ` [new patch] " stardiviner
2021-07-06  4:21             ` Christian Hopps
2021-07-06 15:30               ` [new patch] " stardiviner
2021-07-06 18:23                 ` Christian Hopps
2021-07-08  0:00                   ` stardiviner
2021-07-08  8:59                     ` Christian Hopps
2021-07-08  9:35                       ` STOP this patch for now stardiviner
2021-07-08 12:02                         ` Christian Hopps
2021-07-06 12:13             ` [PATCH] make org-notify support for macOS desktop notification Maxim Nikulin
2021-09-26  8:52             ` [new patch] " Bastien
2021-09-30 15:01               ` Max Nikulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87im1ol2lb.fsf@gmail.com \
    --to=theophilusx@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --subject='Re: [PATCH] make org-notify support for macOS desktop notification' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).