GNU bug report logs - #76120
[PATCH] Expose the native sharing dialog (macOS)

Previous Next

Full log

Message #391 received at 76120 <at> debbugs.gnu.org (full text, mbox):

From: Alvaro Ramirez <alvaro <at> xenodium.com>
To: Visuwesh <visuweshm <at> gmail.com>
Cc: Rudolf Adamkovič <rudolf <at> adamkovic.org>, rms <at> gnu.org,
 Juri Linkov <juri <at> linkov.net>, 76120 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#76120: This feature is not about "sharing", or a "native"
 anything.
Date: Sat, 15 Mar 2025 13:08:49 +0000

Visuwesh <visuweshm <at> gmail.com> writes:

> [செவ்வாய் மார்ச் 04, 2025] Alvaro Ramirez wrote:
>
> Sorry, I was busy with $LIFE so couldn't get a chance to look at 
> the
> latest patch properly.

No worries. Thanks for the comments.

>> +(defun context-menu-send-to (menu _click)
>> +  "Add a \"Send to...\" context MENU entry on supported 
>> platforms."
>> +  (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
>> +  (when (send-to-supported-p)
>> +    (define-key-after menu [separator-send] 
>> menu-bar-separator)
>> +    (define-key-after menu [send]
>> +      '(menu-item "Send to..." (lambda ()
>> +                                 (interactive)
>> +                                 (send-to))
>> +                  :help
>> +                  "Send item (region, buffer file, or Dired 
>> files) to app or service")))
>> +  menu)
>
> Sorry for asking this so late in the game... should we allow the 
> backend
> itself to return a keymap for the context-menu?  This would 
> further
> prompting/choosing "where to send" in some backends.

Can we defer this one as a follow-up feature please? It'd be great 
to get usage feedback before adding more features.

>>  (defun context-menu-buffers (menu _click)
>>    "Populate MENU with the buffer submenus to buffer 
>>    switching."
>>    (run-hooks 'activate-menubar-hook 'menu-bar-update-hook)
>> diff --git a/lisp/send-to.el b/lisp/send-to.el
>> new file mode 100644
>> index 00000000000..064f5ad6645
>> --- /dev/null
>> +++ b/lisp/send-to.el
>> @@ -0,0 +1,233 @@
>> [...]
>> +(defvar-local send-to-handlers '(((:supported 
>> . send-to--ns-supported-p)
>> +                                  (:collect 
>> . send-to--collect-items)
>> +                                  (:send 
>> . send-to--ns-send-items))
>> +                                 ((:supported 
>> . send-to--open-externally-supported-p)
>> +                                  (:collect 
>> . send-to--collect-items)
>> +                                  (:send 
>> . send-to--open-externally)))
>
> I wonder if coupling collect with other items here is a good 
> idea.  We
> could have a separate var for collect functions akin to
> context-menu-functions, that major-mode authors could modify.

I considered that and opted to include as part of the handlers as 
it would possibly simplify/consolidate configuration.

> BTW, seeing the list makes me wonder if using cl-defgeneric for
> support&send functions would be more ergonomic for backend 
> authors.  To
> select the right backend, we could have something like
> xref-backend-functions.

While I didn't consider cl-defgeneric, cl-defstruct did cross my 
mind, and I consciously opted for an alist. Specially when 
debugging, alists are so transparent. Evaluating them gives me 
access to their content without needing additional tooling for 
inspection.

>
>> [...]
>> +;;;###autoload
>> +(defun send-to-supported-p ()
>> +  "Return non-nil for platforms where `send-to' is supported."
>> +  (funcall (map-elt (send-to--resolve-handler) :supported)))
>
> Why not nth?

I typically have a preference for seq.el. I find the consistent 
naming helpful in discovering/finding related functions.

>  I believe it should be a bit faster.

We're likely splitting hairs here. Realistically, this list will 
only ever have a handful of items.

>> [...]
>> +(defun send-to--open-externally (items)
>> +  "Open ITEMS externally (using a non-Emacs application)."
>> +  (unless (boundp 'shell-command-guess-open)
>> +    (require 'dired-aux))
>> +  (when (y-or-n-p (format "Open externally: %s ?"
>> +                          (send-to--format-items items)))
>> +    (dolist (item items)
>> +      (with-temp-buffer
>> +        (unless (zerop (call-process
>> +                        shell-command-guess-open nil 
>> (current-buffer) t
>> +                        (send-to--convert-item-to-filename
>> +                         item)))
>> +          (error "%s" (string-trim (buffer-string))))))))
>> +
>> +(defun send-to--convert-item-to-filename (item)
>> +  "Convert ITEM to a filename.
>> +
>> +Unless ITEM is a verifiable filename, save its content to a 
>> file and
>> +return its new timestamped filename."
>> +  (if (and (file-local-name item) ;; Ignore url-handler-mode
>> +           (file-exists-p item))
>> +      item
>> +    (let ((filename (concat temporary-file-directory
>> +                            (format-time-string "%F_%H.%M.%S") 
>> ".txt")))
>> +      (with-temp-file filename
>> +        (insert item))
>> +      filename)))
>
> Why this renaming?  Wouldn't users want to open the original 
> file
> instead of a copy?

Ah, maybe the docstring needs to be more specific, but the file 
generation is only applicable when sending text that's not in a 
file. For example, sending the current region. This isn't strictly 
necessary for macOS as it can share strings, but I wanted to fill 
the gap for GNU/Linux and offer the same functionality via 
xdg-open. To do that, I need to write the text region to a file 
first.

>> +(defun send-to--collect-items ()
>> +  "Build a list of items to send based on default context.
>> +
>> +From a `dired' buffer, chosen items are based on either of 
>> these being
>> +active:
>> +
>> +  - Marked files
>> +  - Files in region.
>> +  - File at point.
>> +
>> +From any other buffer, either of these two, in order of 
>> preference:
>> +
>> +  - Active region text.
>> +  - Thing at point (via `existing-filename').
>> +  - Buffer file."
>> +  (cond ((derived-mode-p 'dired-mode)
>> +         (or
>> +          (send-to--dired-filenames-in-region)
>> +          (dired-get-marked-files)))
>> +        ((use-region-p)
>> +         (list (buffer-substring-no-properties
>> +                (region-beginning)
>> +                (region-end))))
>> +        ((thing-at-point 'existing-filename)
>> +         (thing-at-point 'existing-filename))
>> +        ((buffer-file-name)
>> +         (list (buffer-file-name)))))
>
> I must sound like a parrot: but could you please consider making
> filenames into file: URLs?  This would avoid the number of 
> file-exists-p
> checks, which is a worthy goal IMO.  Remote files could be 
> checked with
> (file-remote-p default-directory).

Could you share more details about file: please? Sending remote 
files isn't currently a goal. Neither macOS's 
NSSharingServicePicker nor xdg-open (AFAIK) would be able to 
handle remote files?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.