> On Jul 8, 2021, at 4:59 PM, Christian Hopps <cho...@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 <numbch...@gmail.com 
>> <mailto:numbch...@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 <cho...@chopps.org 
>>> <mailto:cho...@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 <numbch...@gmail.com 
>>>> <mailto:numbch...@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 <cho...@chopps.org 
>>>>> <mailto:cho...@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 <numbch...@gmail.com <mailto:numbch...@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 <theophil...@gmail.com 
>>>>>>> <mailto:theophil...@gmail.com>> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> stardiviner <numbch...@gmail.com <mailto:numbch...@gmail.com>> writes:
>>>>>>> 
>>>>>>>>> On Jul 5, 2021, at 7:55 PM, Maxim Nikulin <maniku...@gmail.com 
>>>>>>>>> <mailto:maniku...@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 via email to