Subject: Re: bug#76120: This feature is not about "sharing", or a "native" anything.

From: Visuwesh <[email-masked]>
Date: Sat, 15 Mar 2025 19:06:07 +0530

[சனி மார்ச் 15, 2025] Alvaro Ramirez wrote:

S> Visuwesh <[email-masked]> 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.

Fine by me.

>>>  (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.

OK, sounds good.

>>> [...]
>>> +(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.

Thanks for the clarification; now that I read the code more closely, it
says `insert', not `insert-file-contents'.  My bad.

>>> +(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?

The proposal for file: is mostly so that the backend doesn't have to
second-guess what the item type is.  It could similarly be a cons cell
such as (file . FILENAME).

> Sending remote files isn't currently a goal. Neither macOS's
> NSSharingServicePicker nor xdg-open (AFAIK) would be able to handle
> remote files?

With your current code for the open backend, I think xdg-open will be
executed in the remote machine.  OTOH, one could copy the remote file
using file-local-copy, and call local machine's xdg-open on the local
copy.  I routinely do the latter for opening log files in remote HPCs
with molecular visualisers I have on my laptop:

    (defun vz/dired-do-local-command (command &optional arg files)
      "Run local shell command COMMAND on the marked FILES.
    If ARG is non-nil, then consider run the shell command on the
    next ARG files.  If the region is active, then run the shell
    command on the files in the region.
    FILES should a list of absolute remote filenames.
    For format of COMMAND, refer `dired-do-shell-command'."
      (interactive
       (let ((files (dired-get-marked-files t current-prefix-arg nil nil t)))
         (list (let ((default-directory "~/")) ; To make the completion work.
                 (dired-read-shell-command "! on %s: " current-prefix-arg files))
               current-prefix-arg
               (mapcar #'expand-file-name files)))
       dired-mode)
      (let ((default-directory "~/"))
        (dired-do-shell-command command arg (mapcar (lambda (x) (or (file-local-copy x) x)) files))))