From: Christian Hopps <chopps@chopps.org>
To: stardiviner <numbchild@gmail.com>
Cc: Tim Cross <theophilusx@gmail.com>,
Christian Hopps <chopps@chopps.org>,
Org-mode <emacs-orgmode@gnu.org>
Subject: Re: STOP this patch for now.
Date: Thu, 08 Jul 2021 08:02:19 -0400 [thread overview]
Message-ID: <m25yxlt3hv.fsf@ja.int.chopps.org> (raw)
In-Reply-To: <080251C2-96F5-447A-82A3-7D036A6AA73B@gmail.com>
[-- 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 --]
next prev parent reply other threads:[~2021-07-08 12:11 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-04 0:23 [PATCH] make org-notify support for macOS desktop notification 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
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 [this message]
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=m25yxlt3hv.fsf@ja.int.chopps.org \
--to=chopps@chopps.org \
--cc=emacs-orgmode@gnu.org \
--cc=numbchild@gmail.com \
--cc=theophilusx@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public 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).